Index: includes/common.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/common.inc,v
retrieving revision 1.802
diff -u -9 -p -r1.802 common.inc
--- includes/common.inc	9 Oct 2008 01:49:03 -0000	1.802
+++ includes/common.inc	9 Oct 2008 23:25:47 -0000
@@ -1358,32 +1358,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.
@@ -1397,19 +1391,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 {
@@ -2361,31 +2355,32 @@ 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
+ * - When clean URLs are used, mod_rewrite unescapes %-encoded occurences of the
+ *   characters  % / & # +  which are interpreted as delimiters by PHP. These
  *   characters are double escaped so PHP will still see the encoded version.
  * - 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.137
diff -u -9 -p -r1.137 file.inc
--- includes/file.inc	9 Oct 2008 20:07:00 -0000	1.137
+++ includes/file.inc	9 Oct 2008 23:25:47 -0000
@@ -92,19 +92,26 @@ define('FILE_STATUS_PERMANENT', 1);
  */
 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('\\', '/', $path);
+      if (substr(PHP_OS, 0, 3) == 'WIN') {
+        // PHP on Windows assumes that filenames are encoded in Windows-1252, but
+        // Drupal passes UTF-8-encoded filenames to filesystem functions, so
+        // non-US-ASCII characters end up mangled in the filesystem. We thus need
+        // to do the same mangling to the URLs in order for them to work.
+        $path = utf8_encode($path);
+      }
+      return $GLOBALS['base_url'] . '/' . file_directory_path() . '/' . str_replace(array('%2F', '%5C'), '/', 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.
  *
@@ -651,19 +658,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) {
-  $destination = $directory . '/' . $basename;
+  if (substr(PHP_OS, 0, 3) == 'WIN') {
+    // These characters are not allowed in Windows filenames
+    $basename = str_replace(array(':', '*', '?', '"', '<', '>', '|'), '_', $basename);
+  }
+
+  $destination = strtr($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);
     }
     else {
Index: modules/simpletest/tests/file.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/file.test,v
retrieving revision 1.6
diff -u -9 -p -r1.6 file.test
--- modules/simpletest/tests/file.test	9 Oct 2008 00:02:29 -0000	1.6
+++ modules/simpletest/tests/file.test	9 Oct 2008 23:25:47 -0000
@@ -1129,10 +1129,149 @@ 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
+}
+
+/**
+ * This will test file_create_url() by making round-trips through the web server.
+ */
+class FileCreateUrlTestCase extends FileTestCase {
+  private $files;
+
+  /**
+   * Implementation of getInfo().
+   */
+  function getInfo() {
+    return array(
+      'name' => t('URL generation'),
+      'description' => t('Tests URL generation'),
+      'group' => t('File'),
+    );
+  }
+
+  /**
+   * Implementation of setUp().
+   */
+  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 = ' -._~!$\'"()*@[]?&+%#,;=:' . // ASCII characters, some of which have special meanings in URLs.
+      '%23%25%26%2B%2F%3F' . // Characters that look like a percent-escaped string.
+      'æøåéüöïбвгαβγ中國日本بج'; // Characters from various non-ASCII alphabets.
+    $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%A6%C3%B8%C3%A5%C3%A9%C3%BC%C3%B6%C3%AF%D0%B1%D0%B2%D0%B3%CE%B1%CE%B2%CE%B3%E4%B8%AD%E5%9C%8B%E6%97%A5%E6%9C%AC%D8%A8%D8%AC';
+    $this->checkUrl($file, $expected_url);
+
+    // A file in a subdirectory using backslash as path seperator.
+    $file = 'foo\bar';
+    $expected_url = $base_url . '/' . file_directory_path() . '/foo/bar';
+    $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 = ' -._~!$\'"()*@[]?&+%#,;=:' . // ASCII characters, some of which have special meanings in URLs.
+      '%23%25%26%2B%2F%3F' . // Characters that look like a percent-escaped string.
+      'æøåéüöïбвгαβγ中國日本بج'; // Characters from various non-ASCII alphabets.
+    $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%A6%C3%B8%C3%A5%C3%A9%C3%BC%C3%B6%C3%AF%D0%B1%D0%B2%D0%B3%CE%B1%CE%B2%CE%B3%E4%B8%AD%E5%9C%8B%E6%97%A5%E6%9C%AC%D8%A8%D8%AC';
+    $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);
+
+    // A file in a subdirectory using backslash as path seperator.
+    $file = 'foo\bar';
+    $expected_url = $base_url . '/system/files/foo/bar';
+    $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 = ' -._~!$\'"()*@[]?&+%#,;=:' . // ASCII characters, some of which have special meanings in URLs.
+      '%23%25%26%2B%2F%3F' . // Characters that look like a percent-escaped string.
+      'æøåéüöïбвгαβγ中國日本بج'; // Characters from various non-ASCII alphabets.
+    $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%A6%C3%B8%C3%A5%C3%A9%C3%BC%C3%B6%C3%AF%D0%B1%D0%B2%D0%B3%CE%B1%CE%B2%CE%B3%E4%B8%AD%E5%9C%8B%E6%97%A5%E6%9C%AC%D8%A8%D8%AC';
+    $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);
+
+    // A file in a subdirectory using backslash as path seperator.
+    $file = 'foo\bar';
+    $expected_url = $base_url . '/?q=system/files/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.
+      variable_set('file_test_file_download', $file->filepath);
+    }
+
+    $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);
+  }
+}
Index: modules/simpletest/tests/file_test.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/file_test.module,v
retrieving revision 1.2
diff -u -9 -p -r1.2 file_test.module
--- modules/simpletest/tests/file_test.module	9 Oct 2008 00:02:29 -0000	1.2
+++ modules/simpletest/tests/file_test.module	9 Oct 2008 23:25:47 -0000
@@ -111,18 +111,27 @@ function file_test_file_validate(&$file)
  */
 function file_test_file_status(&$file) {
   $GLOBALS['file_test_results']['status'][] = func_get_args();
 }
 
 /**
  * Implementation of hook_file_download().
  */
 function file_test_file_download(&$file) {
+  // Check whether $file points to the file we just created in
+  // FileCreateUrlTestCase::_testFileCreateUrl().
+  $filepath = file_create_path($file);
+  if ($filepath && $filepath === variable_get('file_test_file_download', FALSE)) {
+    // Return non-empty array to tell file_download() that download is permitted.
+    return array(
+      'X-Foo: Bar',
+    );
+  }
   $GLOBALS['file_test_results']['download'][] = func_get_args();
   return $GLOBALS['file_test_hook_return']['download'];
 }
 
 /**
  * Implementation of hook_file_references().
  */
 function file_test_file_references(&$file) {
   $GLOBALS['file_test_results']['references'][] = func_get_args();