[development] upload module or maybe url() bug

[Matt Connolly]

David - thanks for bringing this back to my attention.

It seems that my fix of including a urlencode() call in  
file_create_url() from '/includes/files.inc'

only works when clean urls is ON.

the original code works correctly when clean_urls is OFF.


I can see now that the offending code is in the function  
"drupal_urlencode()". When clean urls is ON certain items need to be  
double escaped, and the "+" was not one of them when it should have  
been.

I can see two solutions:

1:

     return str_replace(array('%2F', '%26', '%23', '//', '%2B'),
                        array('/', '%2526', '%2523', '/%252F', '%252B'),
                        rawurlencode($text));
2:

     return str_replace(array('%2F', '%', '//'),
                      array('/', '%25', '/%252F'),
                      rawurlencode($text));

Both work for my test files, but (2) seems a bit more generic in that  
anything with a '%' will be double escaped, except for %2F which is  
replaced out first as '/'.

I vote for #2.

anyone else have a preference?

-Matt



On 30/06/2008, at 11:41 PM, David Timothy Strauss wrote:

> We should already be escaping the plus signs:
>
> http://drupal.org/node/191116#comment-627128
>
> ----- "David Timothy Strauss" <david at fourkitchens.com> wrote:
>
>> I'd try an extra step to encode "+" characters in your URLs. I would
>> support adding such a feature to Drupal's url() if it makes the
>> encoding less ambiguous.
>>
>> ----- "Matt Connolly" <matt at cabinetuk.com> wrote:
>>
>>> The + in the file name is being translated into a space and the file
>>>
>>> download does not work because the file is not found.
>>>
>>> When I change the code, as I have, by including a url_encode
>> function
>>>
>>> on the filename, it all works perfectly.
>>>
>>> My question really is: should this be specifically for the file
>>> download - like how I have patched my drupal installation, or is it
>>> better served in the url() function if this would also solve other
>>> urls in the site (page names, etc)
>>>
>>> -Matt
>>>
>>> On 30/06/2008, at 9:16 PM, David Timothy Strauss wrote:
>>>
>>>> See this issue:
>>>> http://drupal.org/node/191116
>>>>
>>>> Translating spaces to "+" is not RFC-compliant. For maximum
>>>> compatibility, you should encode both spaces and plus signs using
>> %
>>>
>>>> notation.
>>>>
>>>> ----- "Matt Connolly" <matt at cabinetuk.com> wrote:
>>>>
>>>>> I have a question about url encoding
>>>>>
>>>>> Spaces in URLS are normally translated to "%20" and spaces in
>>> queries
>>>>>
>>>>> are normally translated into "+", although the "%20" is correctly
>>>>> decoded.
>>>>>
>>>>> I'm discovering that private file uploads are not working when
>>> there
>>>>>
>>>>> is a "+" in the file name. This is being translated into a "
>>>>> " (space)
>>>>>
>>>>> which is quite normal for queries.
>>>>>
>>>>> With public downloads, (ie accessibly directly via
>>>>> http://mydrupalsite/sites/files/fred+wilma.jpg
>>>>> " the file works. So apache doesn't translate the + in the file's
>>>
>>>>> path
>>>>>
>>>>> name into a space, and the file is downloaded.
>>>>>
>>>>> With private downloads, the file name is placed into a query,
>>> thanks
>>>>>
>>>>> to mod_rewrite (or, becomes
>>>>> http://mydrupalsite/q?=system/files/fred+wilma.jpg
>>>>> " . You can see where the problem is.
>>>>>
>>>>>
>>>>> I can easily fix this by altering the "file_create_url" function
>> in
>>>
>>>>> "/
>>>>>
>>>>> includes/file.inc". I know I've been warned to not change
>> drupal's
>>>>> core modules and code, but ..... I think I need to.
>>>>>
>>>>> However, I wonder if the problem would be better handled in the
>>>>> "url()" function?
>>>>>
>>>>> I notice that if you create page url's "bananas+pears" and
>> "apples
>>>
>>>>> and
>>>>>
>>>>> oranges" the urls are all encoded with %20. So it seems you can't
>>> use
>>>>>
>>>>> the "+" in a custom URL. Butt least here what you type and what
>>> you
>>>>> get are consistent.
>>>>>
>>>>>
>>>>> Any thoughts?
>>>>> -Matt