From 9b7f960a2454ad0fc347005fa93c1b84a5255e55 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Thu, 16 Apr 2026 10:18:47 +0100 Subject: [PATCH] serve dlna: fix SOAP response argument ordering for Samsung TV compatibility Samsung TVs are strict DLNA clients that expect SOAP response arguments in the order defined by the service SCPD (Service Control Protocol Description). The Browse response was using a Go map which produces random iteration order, causing arguments like Result, NumberReturned, TotalMatches, and UpdateID to appear in unpredictable order. Samsung TVs fail to parse such responses and never proceed to browse directory children, showing "no content" to the user. Replace the map[string]string return type with an ordered []soapArg slice throughout the UPnPService.Handle() interface, ensuring response arguments always appear in SCPD-defined order. See #9346 --- cmd/serve/dlna/cds.go | 55 +++++++++++++++++++------------------ cmd/serve/dlna/cms.go | 10 +++---- cmd/serve/dlna/dlna.go | 4 +-- cmd/serve/dlna/dlna_test.go | 45 ++++++++++++++++++++++++++++++ cmd/serve/dlna/dlna_util.go | 41 +++++++++++++++++++++------ cmd/serve/dlna/mrrs.go | 14 +++++----- 6 files changed, 121 insertions(+), 48 deletions(-) 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 }