problem with filenames with '%' character [#154245]

I have a client who uploads photos with names like 800_werk%20035.jpg to his site (with image module). Uploading and storing on the server goes fine. Displaying however fails because the html reads: <img src="800_werk%20035.jpg" ...>. When the browser requests this file the server interprets it as 800_werk 035.jpg (%20 decodes as a space) and doesn't find it.
In attachment is a small patch to escape the '%' character in the file_create_url() function.
I don't now if this is the right approach, but it works for my client. Moreover I did not test it with different browsers/platforms.

Comment	File	Size	Author
#10	154245_file_urlencode_04.patch	803 bytes	soxofaan
#9	154245_file_urlencode_03.patch	799 bytes	soxofaan
#6	FILE_DOWNLOADS_PUBLIC-urlencode-file.inc_.patch	703 bytes	toeofdestiny
#5	patch_154245_2.patch	761 bytes	soxofaan
#3	patch_154245.patch	710 bytes	soxofaan
	file.inc-percenturl.patch	778 bytes	soxofaan

Comments

Comment #1

Steven commented 24 June 2007 at 22:19

Status:

Needs review

» Needs work

This need proper urlencoding.

Comment #2

soxofaan commented 25 June 2007 at 12:10

This need proper urlencoding.

That was also my first thought but the problem is that the slash ('/') in images/800_werk%20035.jpg will also be encoded, which is not desired. I guess you need to explode the path on '/' first, urlencode each part and implode again. This is probably a lot of overhead for just coping with a corner case?

Comment #3

soxofaan commented 27 October 2007 at 15:11

Version:	5.1	» 5.x-dev
Status:	Needs work	» Needs review

Status	File	Size
new	patch_154245.patch	710 bytes

This need proper urlencoding.

This patch uses drupal_urlencode() instead of the hackish str_replace() of my original patch.
No problem with "/" characters in the path.

Comment #4

drumm

he/him

NY, US

commented 10 November 2007 at 03:00

Version:	5.x-dev	» 6.x-dev
Status:	Needs review	» Needs work

This would double-encode the path when using the private file download method, since url() does that for us.

Comment #5

soxofaan commented 11 November 2007 at 13:52

Status:

Needs work

» Needs review

Status	File	Size
new	patch_154245_2.patch	761 bytes

moved it inside the FILE_DOWNLOADS_PUBLIC case branch:

function file_create_url($path) {
  // Strip file_directory_path from $path. We only include relative paths in urls.
  if (strpos($path, file_directory_path() . '/') === 0) {
    $path = trim(substr($path, strlen(file_directory_path())), '\\/');
  }
  switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
    case FILE_DOWNLOADS_PUBLIC:
      return $GLOBALS['base_url'] .'/'. file_directory_path() .'/'. str_replace('\\', '/', drupal_urlencode($path));
    case FILE_DOWNLOADS_PRIVATE:
      return url('system/files/'. $path, NULL, NULL, TRUE);
  }
}

At #4 where version was changed from 5.x-def to 6.x dev:
shouldn't this also be fixed in D5?

Comment #6

toeofdestiny commented 3 December 2007 at 22:19

Status	File	Size
new	FILE_DOWNLOADS_PUBLIC-urlencode-file.inc_.patch	703 bytes

I have the same problem here, and here is my patch.

It is almost the same as the already submitted one, except that I "rawurlencode" the path because the paths containing '+' weren't working on my server for some reason.

function file_create_url($path) {
  // Strip file_directory_path from $path. We only include relative paths in urls.
  if (strpos($path, file_directory_path() . '/') === 0) {
    $path = trim(substr($path, strlen(file_directory_path())), '\\/');
  }
  switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
    case FILE_DOWNLOADS_PUBLIC:
      // VVVVVVVVVV PATCH HERE VVVVVVVVVV
      $fullPath = file_directory_path() .'/'. str_replace('\\', '/', $path);
      // Fake "drupal_urlencode" using "rawurlencode" (' ' --> '%20')
      $fullPath = str_replace('+', '%20', drupal_urlencode($fullPath));
      return $GLOBALS['base_url'] .'/'. $fullPath;
      // ^^^^^^^^^^ PATCH HERE ^^^^^^^^^^
    case FILE_DOWNLOADS_PRIVATE:
      return url('system/files/'. $path, NULL, NULL, TRUE);
  }
}