Index: includes/common.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/common.inc,v
retrieving revision 1.835
diff -u -9 -p -r1.835 common.inc
--- includes/common.inc	7 Dec 2008 08:23:07 -0000	1.835
+++ includes/common.inc	8 Dec 2008 21:19:53 -0000
@@ -1604,32 +1604,26 @@ function url($path = NULL, array $option
     if ($options['query']) {
       $path .= (strpos($path, '?') !== FALSE ? '&' : '?') . $options['query'];
     }
     // Reassemble.
     return $path . $options['fragment'];
   }
 
   global $base_url;
   static $script;
-  static $clean_url;
 
   if (!isset($script)) {
     // On some web servers, such as IIS, we can't omit "index.php". So, we
     // generate "index.php?q=foo" instead of "?q=foo" on anything that is not
     // Apache.
     $script = (strpos($_SERVER['SERVER_SOFTWARE'], 'Apache') === FALSE) ? 'index.php' : '';
   }
 
-  // Cache the clean_url variable to improve performance.
-  if (!isset($clean_url)) {
-    $clean_url = (bool)variable_get('clean_url', '0');
-  }
-
   if (!isset($options['base_url'])) {
     // The base_url might be rewritten from the language rewrite in domain mode.
     $options['base_url'] = $base_url;
   }
 
   // Preserve the original path before aliasing.
   $original_path = $path;
 
   // The special path '<front>' links to the default front page.
@@ -1643,19 +1637,19 @@ function url($path = NULL, array $option
   if (function_exists('custom_url_rewrite_outbound')) {
     // Modules may alter outbound links by reference.
     custom_url_rewrite_outbound($path, $options, $original_path);
   }
 
   $base = $options['absolute'] ? $options['base_url'] . '/' : base_path();
   $prefix = empty($path) ? rtrim($options['prefix'], '/') : $options['prefix'];
   $path = drupal_urlencode($prefix . $path);
 
-  if ($clean_url) {
+  if (variable_get('clean_url', '0')) {
     // With Clean URLs.
     if ($options['query']) {
       return $base . $path . '?' . $options['query'] . $options['fragment'];
     }
     else {
       return $base . $path . $options['fragment'];
     }
   }
   else {
@@ -2722,31 +2716,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.146
diff -u -9 -p -r1.146 file.inc
--- includes/file.inc	4 Dec 2008 11:09:33 -0000	1.146
+++ includes/file.inc	8 Dec 2008 21:19:53 -0000
@@ -90,21 +90,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.
  *
@@ -652,18 +671,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.14
diff -u -9 -p -r1.14 file.test
--- modules/simpletest/tests/file.test	27 Nov 2008 08:41:45 -0000	1.14
+++ modules/simpletest/tests/file.test	8 Dec 2008 21:19:53 -0000
@@ -1055,10 +1055,185 @@ class FileSaveDataTest extends FileHookT
     $this->assertTrue($file, t("Unnamed file saved correctly."));
     $this->assertEqual(file_directory_path(), dirname($file->filepath), t("File was placed in Drupal's files directory."));
     $this->assertEqual('asdf.txt', basename($file->filepath), t("File was named correctly."));
     $this->assertEqual($contents, file_get_contents(realpath($file->filepath)), t("Contents of the file are correct."));
 
     // Check the overwrite error.
     $file = file_save_data($contents, 'asdf.txt', FILE_EXISTS_ERROR);
     $this->assertFalse($file, t("Overwriting a file fails when FILE_EXISTS_ERROR is specified."));
   }
-}
\ No newline at end of file
+}
+
+/**
+ * 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());
+    file_check_directory(dirname($filepath), 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);
+  }
+}