Index: includes/common.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/common.inc,v
retrieving revision 1.844
diff -u -9 -p -r1.844 common.inc
--- includes/common.inc	10 Jan 2009 11:50:11 -0000	1.844
+++ includes/common.inc	10 Jan 2009 13:39:03 -0000
@@ -2787,31 +2787,33 @@ function drupal_json($var = NULL) {
  * Wrapper around urlencode() which avoids Apache quirks.
  *
  * Should be used when placing arbitrary data in an URL. Note that Drupal paths
  * are urlencoded() when passed through url() and do not require urlencoding()
  * of individual components.
  *
  * Notes:
  * - For esthetic reasons, we do not escape slashes. This also avoids a 'feature'
  *   in Apache where it 404s on any path containing '%2F'.
- * - mod_rewrite unescapes %-encoded ampersands, hashes, and slashes when clean
- *   URLs are used, which are interpreted as delimiters by PHP. These
- *   characters are double escaped so PHP will still see the encoded version.
+ * - With clean URLs, mod_rewrite unescapes %-encoded characters before
+ *   inserting them into the query string (index.php?q=$1). This causes problems
+ *   with the characters % / & # + that have special meanings in URLs, so these
+ *   are double escaped.
  * - With clean URLs, Apache changes '//' to '/', so every second slash is
  *   double escaped.
  *
  * @param $text
  *   String to encode
  */
 function drupal_urlencode($text) {
   if (variable_get('clean_url', '0')) {
-    return str_replace(array('%2F', '%26', '%23', '//'),
-                       array('/', '%2526', '%2523', '/%252F'),
+    // Decoded:              %        /      &        #        +        //
+    return str_replace(array('%25',   '%2F', '%26',   '%23',   '%2B',   '//'),
+                       array('%2525', '/',   '%2526', '%2523', '%252B', '/%252F'),
                        rawurlencode($text));
   }
   else {
     return str_replace('%2F', '/', rawurlencode($text));
   }
 }
 
 /**
  * Returns a string of highly randomized bytes (over the full 8-bit range).
Index: includes/file.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/file.inc,v
retrieving revision 1.152
diff -u -9 -p -r1.152 file.inc
--- includes/file.inc	10 Jan 2009 06:09:54 -0000	1.152
+++ includes/file.inc	10 Jan 2009 13:39:04 -0000
@@ -82,21 +82,40 @@ define('FILE_STATUS_PERMANENT', 1);
  * @param $path A string containing the path of the file to generate URL for.
  * @return A string containing a URL that can be used to download the file.
  */
 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())), '\\/');
   }
+  if (substr(PHP_OS, 0, 3) == 'WIN') {
+    // On Windows, both "/" and "\" may be used as directory separator, but
+    // use "/" for prettier URLs.
+    $path = strtr($path, '\\', '/');
+  }
   switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
     case FILE_DOWNLOADS_PUBLIC:
-      return $GLOBALS['base_url'] . '/' . file_directory_path() . '/' . str_replace('\\', '/', $path);
+      if (substr(PHP_OS, 0, 3) == 'WIN') {
+        // The filesystem functions in PHP 5 on Windows do not support
+        // UTF-8-encoded filenames but always assume that filenames are encoded
+        // in Windows-1252. In order to support filenames containing characters
+        // not in Windows-1252, Drupal passes UTF-8-encoded filenames to the
+        // filesystem functions, making PHP treat each octet in the
+        // UTF-8-encoded string as a Windows-1252 character. This will make the
+        // filenames look mangled (like viewing an UTF-8-encoded file in a
+        // text editor without support for UTF-8) when viewed with external
+        // programs, e.g. Windows Explorer. The web server does not know about
+        // this convention, so we need to make the URL reflecting the actual
+        // filenames when using public files.
+        $path = drupal_convert_to_utf8($path, 'Windows-1252');
+      }
+      return $GLOBALS['base_url'] . '/' . file_directory_path() . '/' . str_replace('%2F', '/', rawurlencode($path));
     case FILE_DOWNLOADS_PRIVATE:
       return url('system/files/' . $path, array('absolute' => TRUE));
   }
 }
 
 /**
  * Make sure the destination is a complete path and resides in the file system
  * directory, if it is not prepend the file system directory.
  *
@@ -696,18 +715,24 @@ function file_unmunge_filename($filename
  * @param $basename
  *   String filename
  * @param $directory
  *   String directory
  * @return
  *   File path consisting of $directory and a unique filename based off
  *   of $basename.
  */
 function file_create_filename($basename, $directory) {
+  // Strip control characters.
+  $basename = preg_replace('/[\x00-\x1F]/u', '_', $basename);
+  if (substr(PHP_OS, 0, 3) == 'WIN') {
+    // These characters are not allowed in filenames on Windows.
+    $basename = str_replace(array(':', '*', '?', '"', '<', '>', '|'), '_', $basename);
+  }
   $destination = $directory . '/' . $basename;
 
   if (file_exists($destination)) {
     // Destination file already exists, generate an alternative.
     $pos = strrpos($basename, '.');
     if ($pos !== FALSE) {
       $name = substr($basename, 0, $pos);
       $ext = substr($basename, $pos);
     }
Index: modules/simpletest/tests/file.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/file.test,v
retrieving revision 1.20
diff -u -9 -p -r1.20 file.test
--- modules/simpletest/tests/file.test	10 Jan 2009 06:09:54 -0000	1.20
+++ modules/simpletest/tests/file.test	10 Jan 2009 13:39:04 -0000
@@ -1760,9 +1760,184 @@ class FileNameMungingTest extends FileTe
    * Ensure that unmunge gets your name back.
    */
   function testUnMunge() {
     $munged_name = file_munge_filename($this->name, '', FALSE);
     $unmunged_name = file_unmunge_filename($munged_name);
     $this->assertIdentical($unmunged_name, $this->name, t('The unmunged (%unmunged) filename matches the original (%original)', array('%unmunged' => $unmunged_name, '%original' => $this->name)));
   }
 }
 
+/**
+ * Test file_create_url() by making round-trips through the web server.
+ */
+class FileCreateUrlUnitTest extends FileTestCase {
+  function getInfo() {
+    return array(
+      'name' => t('URL generation'),
+      'description' => t('Test URL generation'),
+      'group' => t('File'),
+    );
+  }
+
+  function setUp() {
+    parent::setUp('file_test');
+    // Clear out any hook calls.
+    file_test_reset();
+  }
+
+  /**
+   * Test file_create_url() using FILE_DOWNLOADS_PUBLIC.
+   */
+  function testFileCreateUrlPublic() {
+    global $base_url;
+    variable_set('file_downloads', FILE_DOWNLOADS_PUBLIC);
+
+    $file = " -._~!$'\"()*@[]?&+%#,;=:\n\x00" . // "Special" ASCII characters.
+      "%23%25%26%2B%2F%3F" . // Characters that look like a percent-escaped string.
+      "éøïвβ中國書۞"; // Characters from various non-ASCII alphabets.
+    if (substr(PHP_OS, 0, 3) == 'WIN') {
+      $expected_url = $base_url . '/' . file_directory_path() . '/' .
+        '%20-._%7E%21%24%27_%28%29_%40%5B%5D_%26%2B%25%23%2C%3B%3D___' .
+        '%2523%2525%2526%252B%252F%253F' .
+        '%C3%83%C2%A9%C3%83%C2%B8%C3%83%C2%AF%C3%90%C2%B2%C3%8E%C2%B2%C3%A4%C2%B8%C2%AD%C3%A5%C5%93%E2%80%B9%C3%A6%E2%80%BA%C2%B8%C3%9B%C5%BE';
+    }
+    else {
+      $expected_url = $base_url . '/' . file_directory_path() . '/' .
+        '%20-._%7E%21%24%27%22%28%29%2A%40%5B%5D%3F%26%2B%25%23%2C%3B%3D%3A__' .
+        '%2523%2525%2526%252B%252F%253F' .
+        '%C3%A9%C3%B8%C3%AF%D0%B2%CE%B2%E4%B8%AD%E5%9C%8B%E6%9B%B8%DB%9E';
+    }
+    $this->checkUrl($file, $expected_url);
+
+    // On Windows, "\" works as path separator; on other platforms it is
+    // treated as any other character.
+    $file = 'foo\bar';
+    if (substr(PHP_OS, 0, 3) == 'WIN') {
+      $expected_url = $base_url . '/' . file_directory_path() . '/foo/bar';
+    }
+    else {
+      $expected_url = $base_url . '/' . file_directory_path() . '/foo%5Cbar';
+    }
+    $this->checkUrl($file, $expected_url);
+  }
+
+  /**
+   * Test file_create_url() using FILE_DOWNLOADS_PRIVATE and clean URLs enabled.
+   */
+  function testFileCreateUrlPrivateCleanUrlEnabled() {
+    global $base_url;
+    variable_set('clean_url', '1');
+    variable_set('file_downloads', FILE_DOWNLOADS_PRIVATE);
+
+    $file = " -._~!$'\"()*@[]?&+%#,;=:\n\x00" . // "Special" ASCII characters.
+      "%23%25%26%2B%2F%3F" . // Characters that look like a percent-escaped string.
+      "éøïвβ中國書۞"; // Characters from various non-ASCII alphabets.
+    if (substr(PHP_OS, 0, 3) == 'WIN') {
+      $expected_url = $base_url . '/system/files/' .
+        '%20-._%7E%21%24%27_%28%29_%40%5B%5D_%2526%252B%2525%2523%2C%3B%3D___' .
+        '%252523%252525%252526%25252B%25252F%25253F' .
+        '%C3%A9%C3%B8%C3%AF%D0%B2%CE%B2%E4%B8%AD%E5%9C%8B%E6%9B%B8%DB%9E';
+    }
+    else {
+      $expected_url = $base_url . '/system/files/' .
+        '%20-._%7E%21%24%27%22%28%29%2A%40%5B%5D%3F%2526%252B%2525%2523%2C%3B%3D%3A__' .
+        '%252523%252525%252526%25252B%25252F%25253F' .
+        '%C3%A9%C3%B8%C3%AF%D0%B2%CE%B2%E4%B8%AD%E5%9C%8B%E6%9B%B8%DB%9E';
+    }
+    $this->checkUrl($file, $expected_url);
+
+    // "0" is tricky because "0" == FALSE.
+    $file = '0';
+    $expected_url = $base_url . '/system/files/0';
+    $this->checkUrl($file, $expected_url);
+
+    // On Windows, "\" works as path separator; on other platforms it is
+    // treated as any other character.
+    $file = 'foo\bar';
+    if (substr(PHP_OS, 0, 3) == 'WIN') {
+      $expected_url = $base_url . '/system/files/foo/bar';
+    }
+    else {
+      $expected_url = $base_url . '/system/files/foo%5Cbar';
+    }
+    $this->checkUrl($file, $expected_url);
+  }
+
+  /**
+   * Test file_create_url() using FILE_DOWNLOADS_PRIVATE and clean URLs
+   * disabled.
+   */
+  function testFileCreateUrlPrivateCleanUrlDisabled() {
+    global $base_url;
+    variable_set('clean_url', '0');
+    variable_set('file_downloads', FILE_DOWNLOADS_PRIVATE);
+
+    $file = " -._~!$'\"()*@[]?&+%#,;=:\n\x00" . // "Special" ASCII characters.
+      "%23%25%26%2B%2F%3F" . // Characters that look like a percent-escaped string.
+      "éøïвβ中國書۞"; // Characters from various non-ASCII alphabets.
+    if (substr(PHP_OS, 0, 3) == 'WIN') {
+      $expected_url = $base_url . '/?q=system/files/' .
+        '%20-._%7E%21%24%27_%28%29_%40%5B%5D_%26%2B%25%23%2C%3B%3D___' .
+        '%2523%2525%2526%252B%252F%253F' .
+        '%C3%A9%C3%B8%C3%AF%D0%B2%CE%B2%E4%B8%AD%E5%9C%8B%E6%9B%B8%DB%9E';
+    }
+    else {
+      $expected_url = $base_url . '/?q=system/files/' .
+        '%20-._%7E%21%24%27%22%28%29%2A%40%5B%5D%3F%26%2B%25%23%2C%3B%3D%3A__' .
+        '%2523%2525%2526%252B%252F%253F' .
+        '%C3%A9%C3%B8%C3%AF%D0%B2%CE%B2%E4%B8%AD%E5%9C%8B%E6%9B%B8%DB%9E';
+    }
+    $this->checkUrl($file, $expected_url);
+
+    // "0" is tricky because "0" == FALSE.
+    $file = '0';
+    $expected_url = $base_url . '/?q=system/files/0';
+    $this->checkUrl($file, $expected_url);
+
+    // On Windows, "\" works as path separator; on other platforms it is
+    // treated as any other character.
+    $file = 'foo\bar';
+    if (substr(PHP_OS, 0, 3) == 'WIN') {
+      $expected_url = $base_url . '/?q=system/files/foo/bar';
+    }
+    else {
+      $expected_url = $base_url . '/?q=system/files/foo%5Cbar';
+    }
+    $file = 'foo\bar';
+    $this->checkUrl($file, $expected_url);
+  }
+
+  /**
+   * Check that the URL generated by file_create_url() for the specified file
+   * equals the specified URL, then fetch the URL and compare the contents to
+   * the file.
+   *
+   * @param $path
+   *   A filepath.
+   * @param $expected_url
+   *   The expected URL.
+   */
+  private function checkUrl($path, $expected_url) {
+    // Convert $path to a valid filename, i.e. strip characters not supported
+    // by the filesystem, and create the file.
+    $filepath = file_create_filename($path, file_directory_path());
+    $directory = dirname($filepath); 
+    file_check_directory($directory, FILE_CREATE_DIRECTORY);
+    $file = $this->createFile($filepath);
+
+    $url = file_create_url($file->filepath);
+    $this->assertEqual($url, $expected_url, t('Generated URL matches expected URL'));
+
+    if (variable_get('file_downloads', FALSE) == FILE_DOWNLOADS_PRIVATE) {
+      // Tell the implementation of hook_file_download() in file_test.module
+      // that this file may be downloaded.
+      file_test_set_return('download', array('X-Foo: Bar'));
+    }
+
+    $this->drupalGet($url);
+    if ($this->assertResponse(200) == 'pass') {
+      $this->assertRaw(file_get_contents($file->filepath), t('Contents of the file are correct.'));
+    }
+
+    file_delete($file);
+  }
+}