filenames with question marks are not scanned, patch to fix!

[Anonymous]

Hi all,

I’ve just installed the nightly build 1696 after not having much luck with the last stable.. It is much more stable on my system! Oh well, I like living on the edge.

I have a fairly larged but very old MP3 collection and I named all the files with question marks as required. However, it seems that the call to io_open in scan_mp3 is formulating a url as follows:

file://MP3s/The+Smiths/Hatful+Of+Hollow/The+Smiths+-+How+Soon+Is+Now?.mp3

Seeing as the filename is really:
/MP3s/The Smiths/Hatful Of Hollow/The Smiths – How Soon Is Now?.mp3

io_open is treating the ? as the start of an option just as a HTTP url would have http://blah.com/search?item=mp3s

Problem is, that means it is actually trying to open:
MP3s/The Smiths/Hatful Of Hollow/The Smiths – How Soon Is Now

Leaving “?.mp3” off the end!

I must admit I’ve actually spent longer writing this post than I have looking at it. I intend to find some more info and may a patch to solve the problem. I’m guessing the filename component during the “HTTP”ization is missing the ? (but is processing 0x20 correctly into a ‘+’)

I’ll do some more digging but thought you guys might want to know

Cheers

cp

[S80_UK]

Hi,

I didn’t notice which operating system you are using. In many OS’s the question mark is not a valid character for use in file names (along with / : and *). In the case of the question mark it is because it acts as a single character “wild-card” when searching for a file. So a search for fred?.mp3 could actually access fred1.mp3 fred9.mp3 freda.mp3 or fredz.mp3 (just a few of many possibilities).

The best advice I can give would be to rename filenames containing ? to something more widely compatible.

[Anonymous]

I’m using unix. The fact is the process of turning a ? into a file is called globbing, this is the same as “*”. Globbing is NOT part of file access. Globbing is performed on a value to turn it into a file name that can be opened. This is not the fault here

The fault here (IMHO) is in io_open in io.c

It is mistakenly interpreting the ? as a option seperator.. I understand your point, but lets face it Windows might have problems with colons and ? and *, but Unix has a problem with /

If you are multiplatform these issues should be taken into account. Some OS don’t support sub directories, but that is no reason to remove support for them! You can’t just adopt the lower common denominator!

[Anonymous]

Ok, more light on the subject..

In io.c:io_open()
io_open is pass the location: file:///MP3s/The+Smiths/Hatful+Of+Hollow/The+Smiths+-+How+Soon+Is+Now%3f.mp3

1) Split the location into protocol and path
proto_part=file
path_part=/MP3s/The+Smiths/Hatful+Of+Hollow/The+Smiths+-+How+Soon+Is+Now%3f.mp3

2) URL decode the path
proto_part=file
path_part=/MP3s/The Smiths/Hatful Of Hollow/The Smiths – How Soon Is Now?.mp3

3) Split the path at ? for options
path_part=MP3s/The Smiths/Hatful Of Hollow/The Smiths – How Soon Is Now
option_part=.mp3

I don’t believe this to be correct

Take this URL for example

http://somesite/path+with+spaces/question%3fmark

The %3f is not decoded THEN interpreted as a option seperated. The ? must be searched for prior to URL decoding
Url decoding can then be carried out AFTER the ? has been located..

I think:
if(path_part)
io_urldecode(path_part);
needs to happen AFTER the option_part have been defined and on option_part and path_part independently

This way an encoded ? as %3f will not be incorrectly parsed as an option.

I’ll write a patch after I’ve tested this theory out!

[Anonymous]

Ok, here’s a patch. I’ve just compiled and it no longer complains about files with question marks! yeay!

— mt-daapd-svn-1696/src/io.c 2007-10-29 06:02:27.000000000 +0000
+++ mt-daapd-svn-1696-io_open_question_mark/src/io.c 2008-02-02 18:29:22.000000000 +0000
@@ -664,15 +664,16 @@
proto_part = uri_copy;
}

– if(path_part)
– io_urldecode(path_part);

/* find the start of the options */
options_part = strchr(path_part,’?’);
if(options_part) {
*options_part = ”;
options_part++;
+ io_urldecode(options_part);
}
+ if(path_part)
+ io_urldecode(path_part);

/* see if we can generate a list of options */
while(options_part) {

My only concern is that some other user of io_open may double encode a url with options in order to work around the original bug

http://some/site?search=string
would be converted into
http://some/site%3fsearch=string
for example, which is bad! Can’t think why that should be the case, but it might be worth thinking about

Either way, this patch appears to solve my problem quite nicely

[Anonymous]

Patch also available at:

http://chris.intrepid.cx/mt-daapd-svn-1696-io_open_question_mark.diff

[S80_UK]

Wow… Like I said, “My best advice…”

Clearly you know much more about this than I. Thanks for sharing, and thanks for posting the fix.

Les.

[mas]

Pls send this patch also to Ron in case he overlooks this thread.

[rpedde]

@cpitchford wrote:

My only concern is that some other user of io_open may double encode a url with options in order to work around the original bug

I thought about that myself. The only place that would have input that gets passed to io_open that could be double-encoded would be from the admin interface, and that should only get decoded once, so it will fail trying to open the (single encoded) filename.

That is, if the path/options part is split first, and path and options are urldecoded seperately. Which is how it should be done, I believe.

— Ron

[ghedo]

the path to solve albums and with ‘?’ seem not included….

[Author]

Posts