Bug 16441 - XMLBrowser search - does not escape utf8 strings in url correctly
: XMLBrowser search - does not escape utf8 strings in url correctly
Status: RESOLVED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: XML
: 7.5.x
: PC Windows (legacy)
: -- normal (vote)
: ---
Assigned To: Adrian Smith
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-07 04:03 UTC by Adrian Smith
Modified: 2011-03-16 04:47 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Smith 2010-08-07 04:03:32 UTC
Noticed this with my spotify plugin - xmlbrowser will make requests which don't correctly escape utf8 strings in search params.  Not sure where it is best to call URI::Escape::uri_escape_utf8 hence recording here.

Alan - assuming you may want to check this as you have been looking at xmlbrowser recently.
Comment 1 Adrian Smith 2010-08-08 04:14:39 UTC
I think we need something like:

Index: Slim/Web/XMLBrowser.pm
===================================================================
--- Slim/Web/XMLBrowser.pm      (revision 31149)
+++ Slim/Web/XMLBrowser.pm      (working copy)
@@ -294,7 +294,7 @@
 
                                # Rewrite the URL if it was a search request
                                if ( $subFeed->{'type'} eq 'search' && ( $stash->{'q'} || $searchQuery ) ) {
-                                       my $search = $stash->{'q'} || $searchQuery;
+                                       my $search = URI::Escape::uri_escape_utf8($stash->{'q'} || $searchQuery);
                                        $subFeed->{'url'} =~ s/{QUERY}/$search/g;
                                }

This is in addition to escaping the breadcrumb - we need to ensure we send the correct search terms to the http server and this means escaping them.
Comment 2 Adrian Smith 2010-10-14 12:46:14 UTC
Andy - do you agree with this patch?

Several people have been complaining about this with my spotify plugin and I want to fix it.  There doesn't seem to be any more work going on with onebrowser, so I would like to fix 7.6 (and possibly 7.5 if you are planning another release)
Comment 3 Andy Grundman 2010-10-14 13:56:06 UTC
Yes patch is fine, thanks!
Comment 4 SVN Bot 2010-10-17 03:30:14 UTC
 == Auto-comment from SVN commit #31445 to the slim repo by adrian ==
 == http://svn.slimdevices.com/slim?view=revision&revision=31445 ==

Bug: 16441
Description: uri escape search strings in xmlbrowser
Comment 5 Alan Young 2011-02-10 04:21:32 UTC
Is this fixed now?
Comment 6 Adrian Smith 2011-02-12 04:52:44 UTC
This is working in onebrowse, but the breadcrumb for unicode searches loses the encoding when you browse into the search item.

I think this fixes it - if you agree should be in 7.6 and onebrowse (ideally 7.5 too if there is any more life in this!)

--- Slim/Web/XMLBrowser.pm      (revision 31902)
+++ Slim/Web/XMLBrowser.pm      (working copy)
@@ -279,6 +279,7 @@
                        }
                        elsif ( $i =~ /(?:\d+)?_(.+)/ ) {
                                $searchQuery = uri_unescape($1);
+                               Slim::Utils::Unicode::utf8on($searchQuery);
                        }
 
                        # Add search query to crumbName
Comment 7 Alan Young 2011-02-12 08:33:14 UTC
Yes, I think that Slim::Utils::Unicode::utf8on() should always be considered as a candidate to be called on the results of uri_unescape, if there is any chance that the unescaped string may contain UTF-8.
Comment 8 SVN Bot 2011-02-13 00:37:50 UTC
 == Auto-comment from SVN commit #31903 to the slim repo by ayoung ==
 == http://svn.slimdevices.com/slim?view=revision&revision=31903 ==

bug 16441: XMLBrowser search - does not escape utf8 strings in url correctly 
Set UTF8 flag after unescaping.
Comment 9 Alan Young 2011-02-13 00:39:27 UTC
It seems to work ok for me in 7.6 trunk. Adrian, do you still see any problems there?
Comment 10 Adrian Smith 2011-02-13 02:36:22 UTC
Yes - same case works ok in 7.6, lets call this closed.  Thanks.