diff --git a/cmd/serve/dlna/cds.go b/cmd/serve/dlna/cds.go index 22d9d4d32..c9a87b488 100644 --- a/cmd/serve/dlna/cds.go +++ b/cmd/serve/dlna/cds.go @@ -266,18 +266,18 @@ func (cds *contentDirectoryService) objectFromID(id string) (o object, err error return } -func (cds *contentDirectoryService) Handle(action string, argsXML []byte, r *http.Request) (map[string]string, error) { +func (cds *contentDirectoryService) Handle(action string, argsXML []byte, r *http.Request) ([]soapArg, error) { host := r.Host switch action { case "GetSystemUpdateID": - return map[string]string{ - "Id": cds.updateIDString(), - }, nil + return soapArgs( + "Id", cds.updateIDString(), + ), nil case "GetSortCapabilities": - return map[string]string{ - "SortCaps": "dc:title", - }, nil + return soapArgs( + "SortCaps", "dc:title", + ), nil case "Browse": var browse browse if err := xml.Unmarshal(argsXML, &browse); err != nil { @@ -305,12 +305,13 @@ func (cds *contentDirectoryService) Handle(action string, argsXML []byte, r *htt if err != nil { return nil, err } - return map[string]string{ - "TotalMatches": fmt.Sprint(totalMatches), - "NumberReturned": fmt.Sprint(len(objs)), - "Result": didlLite(string(result)), - "UpdateID": cds.updateIDString(), - }, nil + // Argument order must match SCPD definition in ContentDirectory.xml + return soapArgs( + "Result", didlLite(string(result)), + "NumberReturned", fmt.Sprint(len(objs)), + "TotalMatches", fmt.Sprint(totalMatches), + "UpdateID", cds.updateIDString(), + ), nil case "BrowseMetadata": node, err := cds.vfs.Stat(obj.Path) if err != nil { @@ -325,32 +326,34 @@ func (cds *contentDirectoryService) Handle(action string, argsXML []byte, r *htt if err != nil { return nil, err } - return map[string]string{ - "TotalMatches": "1", - "NumberReturned": "1", - "Result": didlLite(string(result)), - "UpdateID": cds.updateIDString(), - }, nil + // Argument order must match SCPD definition in ContentDirectory.xml + return soapArgs( + "Result", didlLite(string(result)), + "NumberReturned", "1", + "TotalMatches", "1", + "UpdateID", cds.updateIDString(), + ), nil default: return nil, upnp.Errorf(upnp.ArgumentValueInvalidErrorCode, "unhandled browse flag: %v", browse.BrowseFlag) } case "GetSearchCapabilities": - return map[string]string{ - "SearchCaps": "", - }, nil + return soapArgs( + "SearchCaps", "", + ), nil // Samsung Extensions case "X_GetFeatureList": - return map[string]string{ - "FeatureList": ` + return soapArgs( + "FeatureList", ` -`}, nil +`, + ), nil case "X_SetBookmark": // just ignore - return map[string]string{}, nil + return nil, nil default: return nil, upnp.InvalidActionError } diff --git a/cmd/serve/dlna/cms.go b/cmd/serve/dlna/cms.go index 1edce9c36..7f69350d9 100644 --- a/cmd/serve/dlna/cms.go +++ b/cmd/serve/dlna/cms.go @@ -13,13 +13,13 @@ type connectionManagerService struct { upnp.Eventing } -func (cms *connectionManagerService) Handle(action string, argsXML []byte, r *http.Request) (map[string]string, error) { +func (cms *connectionManagerService) Handle(action string, argsXML []byte, r *http.Request) ([]soapArg, error) { switch action { case "GetProtocolInfo": - return map[string]string{ - "Source": defaultProtocolInfo, - "Sink": "", - }, nil + return soapArgs( + "Source", defaultProtocolInfo, + "Sink", "", + ), nil default: return nil, upnp.InvalidActionError } diff --git a/cmd/serve/dlna/dlna.go b/cmd/serve/dlna/dlna.go index 84068049d..bfa4a464b 100644 --- a/cmd/serve/dlna/dlna.go +++ b/cmd/serve/dlna/dlna.go @@ -252,7 +252,7 @@ func newServer(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Opt // UPnPService is the interface for the SOAP service. type UPnPService interface { - Handle(action string, argsXML []byte, r *http.Request) (respArgs map[string]string, err error) + Handle(action string, argsXML []byte, r *http.Request) (respArgs []soapArg, err error) Subscribe(callback []*url.URL, timeoutSeconds int) (sid string, actualTimeout int, err error) Unsubscribe(sid string) error } @@ -327,7 +327,7 @@ func (s *server) serviceControlHandler(w http.ResponseWriter, r *http.Request) { } // Handle a SOAP request and return the response arguments or UPnP error. -func (s *server) soapActionResponse(sa upnp.SoapAction, actionRequestXML []byte, r *http.Request) (map[string]string, error) { +func (s *server) soapActionResponse(sa upnp.SoapAction, actionRequestXML []byte, r *http.Request) ([]soapArg, error) { service, ok := s.services[sa.Type] if !ok { // TODO: What's the invalid service error? diff --git a/cmd/serve/dlna/dlna_test.go b/cmd/serve/dlna/dlna_test.go index 6dea80919..cfef1a555 100644 --- a/cmd/serve/dlna/dlna_test.go +++ b/cmd/serve/dlna/dlna_test.go @@ -136,6 +136,51 @@ func TestContentDirectoryBrowseMetadata(t *testing.T) { require.Contains(t, string(body), html.EscapeString("")) } +// Check that Browse response arguments are in the SCPD-defined order. +// Samsung TVs require this specific ordering to work correctly. +// See: https://github.com/rclone/rclone/issues/9346 +func TestContentDirectoryBrowseResponseOrder(t *testing.T) { + req, err := http.NewRequest("POST", baseURL+serviceControlURL, strings.NewReader(` + + + + + 0 + BrowseMetadata + * + 0 + 0 + + + +`)) + require.NoError(t, err) + req.Header.Set("SOAPACTION", `"urn:schemas-upnp-org:service:ContentDirectory:1#Browse"`) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + bodyStr := string(body) + + // Verify that the response arguments appear in SCPD-defined order: + // Result, NumberReturned, TotalMatches, UpdateID + resultIdx := strings.Index(bodyStr, "") + numberReturnedIdx := strings.Index(bodyStr, "") + totalMatchesIdx := strings.Index(bodyStr, "") + updateIDIdx := strings.Index(bodyStr, "") + + require.NotEqual(t, -1, resultIdx, "Result element not found") + require.NotEqual(t, -1, numberReturnedIdx, "NumberReturned element not found") + require.NotEqual(t, -1, totalMatchesIdx, "TotalMatches element not found") + require.NotEqual(t, -1, updateIDIdx, "UpdateID element not found") + + assert.Less(t, resultIdx, numberReturnedIdx, "Result should come before NumberReturned") + assert.Less(t, numberReturnedIdx, totalMatchesIdx, "NumberReturned should come before TotalMatches") + assert.Less(t, totalMatchesIdx, updateIDIdx, "TotalMatches should come before UpdateID") +} + // Check that the X_MS_MediaReceiverRegistrar is faked out properly. func TestMediaReceiverRegistrarService(t *testing.T) { env := soap.Envelope{ diff --git a/cmd/serve/dlna/dlna_util.go b/cmd/serve/dlna/dlna_util.go index 7eabdd50e..4cf166959 100644 --- a/cmd/serve/dlna/dlna_util.go +++ b/cmd/serve/dlna/dlna_util.go @@ -77,17 +77,42 @@ func mustMarshalXML(value any) []byte { return ret } +// soapArg is an ordered SOAP response argument. +type soapArg struct { + name string + value string +} + +// soapArgs creates a list of soapArg from pairs of name, value strings. +// Panics if an odd number of strings is provided. +func soapArgs(nameValuePairs ...string) []soapArg { + if len(nameValuePairs)%2 != 0 { + fs.Panicf(nil, "soapArgs: odd number of arguments") + } + args := make([]soapArg, len(nameValuePairs)/2) + for i := range args { + args[i] = soapArg{ + name: nameValuePairs[i*2], + value: nameValuePairs[i*2+1], + } + } + return args +} + // Marshal SOAP response arguments into a response XML snippet. -func marshalSOAPResponse(sa upnp.SoapAction, args map[string]string) []byte { - soapArgs := make([]soap.Arg, 0, len(args)) - for argName, value := range args { - soapArgs = append(soapArgs, soap.Arg{ - XMLName: xml.Name{Local: argName}, - Value: value, - }) +// Argument order is preserved from the input slice, which is important +// for compatibility with strict DLNA clients like Samsung TVs that +// expect arguments in the order defined by the service SCPD. +func marshalSOAPResponse(sa upnp.SoapAction, args []soapArg) []byte { + xmlArgs := make([]soap.Arg, len(args)) + for i, arg := range args { + xmlArgs[i] = soap.Arg{ + XMLName: xml.Name{Local: arg.name}, + Value: arg.value, + } } return fmt.Appendf(nil, `%[3]s`, - sa.Action, sa.ServiceURN.String(), mustMarshalXML(soapArgs)) + sa.Action, sa.ServiceURN.String(), mustMarshalXML(xmlArgs)) } type loggingResponseWriter struct { diff --git a/cmd/serve/dlna/mrrs.go b/cmd/serve/dlna/mrrs.go index 70061bd71..c5251717e 100644 --- a/cmd/serve/dlna/mrrs.go +++ b/cmd/serve/dlna/mrrs.go @@ -11,16 +11,16 @@ type mediaReceiverRegistrarService struct { upnp.Eventing } -func (mrrs *mediaReceiverRegistrarService) Handle(action string, argsXML []byte, r *http.Request) (map[string]string, error) { +func (mrrs *mediaReceiverRegistrarService) Handle(action string, argsXML []byte, r *http.Request) ([]soapArg, error) { switch action { case "IsAuthorized", "IsValidated": - return map[string]string{ - "Result": "1", - }, nil + return soapArgs( + "Result", "1", + ), nil case "RegisterDevice": - return map[string]string{ - "RegistrationRespMsg": mrrs.RootDeviceUUID, - }, nil + return soapArgs( + "RegistrationRespMsg", mrrs.RootDeviceUUID, + ), nil default: return nil, upnp.InvalidActionError }