[mythtv-commits] Ticket #13315: Use fully-decoded URL paths when translating to local paths

[MythTV]

#13315: Use fully-decoded URL paths when translating to local paths
----------------------------------+-------------------------
     Reporter:  ijc               |      Owner:  (none)
         Type:  Patch - Bug Fix   |     Status:  new
     Priority:  minor             |  Milestone:  30.0
    Component:  MythTV - General  |    Version:  Master Head
     Severity:  medium            |   Keywords:
Ticket locked:  0                 |
----------------------------------+-------------------------
 Please see https://github.com/MythTV/mythtv/pull/167.
 Not quite sure about the component, is this services API backend? I left
 as general.
 I set the version/milestone to master & 30, but it would be great to get
 this back ported to fixes/29 too please.

 Quoting the PR description:

 Previously it was not possible to access files with certain characters in
 them
 remotely (such as music tracks with quotes in the title) because the
 characters
 are URL %-escaped, e.g.:

 {{{
         ProcessRequest storagegroup.cpp:608 (FindFile) SG(Music):
 FindFile: Searching for '/Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG(Music):
 FindFileDir: Checking '/storage/music' for '/storage/music//Enslaved -
 Isa/01-Intro: %22Green Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG(Default):
 FindFileDir: Checking '/var/lib/mythtv/recordings' for
 '/var/lib/mythtv/recordings//Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/lib/mythtv/.mythtv/Banners' for
 '/var/lib/mythtv/.mythtv/Banners//Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/lib/mythtv/.mythtv/Coverart' for
 '/var/lib/mythtv/.mythtv/Coverart//Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/backups/mythtv' for
 '/var/backups/mythtv//Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/lib/mythtv/recordings' for
 '/var/lib/mythtv/recordings//Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/lib/mythtv/.mythtv/Fanart' for
 '/var/lib/mythtv/.mythtv/Fanart//Enslaved - Isa/01-Intro: %22Gree
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/storage/music' for '/storage/music//Enslaved -
 Isa/01-Intro: %22Green Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/lib/mythtv/.mythtv/MusicArt' for
 '/var/lib/mythtv/.mythtv/MusicArt//Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/storage/media' for '/storage/media//Enslaved -
 Isa/01-Intro: %22Green Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/lib/mythtv/.mythtv/Screenshots' for
 '/var/lib/mythtv/.mythtv/Screenshots//Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/lib/mythtv/.mythtv/Trailers' for
 '/var/lib/mythtv/.mythtv/Trailers//Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'
         ProcessRequest storagegroup.cpp:639 (FindFileDir) SG():
 FindFileDir: Checking '/var/lib/mythvideo' for
 '/var/lib/mythvideo//Enslaved - Isa/01-Intro: %22Green Reflection%22.flac'
         ProcessRequest storagegroup.cpp:622 (FindFile) SG(Music):
 FindFile: Unable to find '/Enslaved - Isa/01-Intro: %22Green
 Reflection%22.flac'!
         ProcessRequest mainserver.cpp:7968 (LocalFilePath) MainServer:
 ERROR: LocalFilePath unable to find local path for '/Enslaved -
 Isa/01-Intro: %22Green Reflection%22.flac'.
 }}}

 This is because the actual file on disk is not %-encoded, the actual
 filename
 is `01-Intro: "Green Reflection".flac`. If the filename had been
 %-encoded,
 then the resulting %'s would, I beleive, have themselves been encoded as
 %25 by
 this point.

 To fix this switch from `url.toString()` to
 `url.path(QUrl::FullyDecoded)`.
 Note that:
     - `url.toString()` does not accept `QUrl::FullyDecoded`. That produces
 the
       error message:
       `QUrl: QUrl::FullyDecoded is not permitted when reconstructing the
 full URL`
     - at this point in the code the URL only contains a path element and a
 possible
       fragment (handled separately in the code). The is no `scheme://` or
 hostname
       etc). I think the only reason it is a `QUrl` at this point and not a
       `QString` is the fragment handling.
     - a path (not a full URL) is what `StorageGroup::FindFile` and friends
 expect,
       they take `QString` not `QUrl`.

 Also add `QUrl::FullyDecoded` to some existing uses of `url.path()` in
 these
 code paths.

 There seems to be two very similar copies of this code (libmythprotoserver
 and
 in mythbackend/mainserver directly) so I have adjusted both. It's not
 entirely
 clear to me where they are each used from, but I have tested (on a
 backport to
 fixes/29):

     - recording playback (both preexisting recordings and live TV), the
 filenames
       here are autogenerated and are not expected to contain any quotable
       characters AFAIK.
     - mythvideo (filenames with and without quote characters).
     - mythmusic (filenames with and without quote characters).
     - old gallery plugin (only filenames without quotes).

 Signed-off-by: Ian Campbell <ijc at hellion.org.uk>

-- 
Ticket URL: <https://code.mythtv.org/trac/ticket/13315>
MythTV <http://www.mythtv.org>
MythTV Media Center