Bug 30225 (CVE-2012-3696)

Summary: window.location.href and others needlessly decodes URI-encoded characters
Product: WebKit Reporter: Andrei Popescu <andreip@google.com>
Component: HTML DOMAssignee: Erik Arvidsson <arv@chromium.org>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth@webkit.org, ap@webkit.org, arv@chromium.org, ayao@apple.com, benm@google.com, darin@apple.com, dglazkov@chromium.org, ericu@chromium.org, evn@google.com, fishd@chromium.org, hudsonr@us.ibm.com, jashkenas@gmail.com, jeffcz@apple.com, michiel@parse.nl, ossy@webkit.org, rniwa@webkit.org, ryan@wonko.com, sam@samuelcole.name, steveblock@chromium.org, tom@eggdrop.ch, webkit.review.bot@gmail.com
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 69511    
Bug Blocks:    
Attachments:
Description Flags
Make KURL::prettyUrl (used by window.location.href) not URI-decode the URI path.
none
Make KURL::prettyUrl (used by window.location.href) not URI-decode the URI path.
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing none

Description From 2009-10-08 11:20:05 PST
Navigate to a URL that contains URI escape sequences in its path, e.g. http://example.com/foo%3dbar and query window.location.href. This property should return the URI unmodified in any way. What actually happens is that the property contains the URI with its path component being URI-decoded, i.e. http://example.com/foo=bar. 

IE, Mozilla FF, Chrome all return the path unmodified. Safari 4.x and the Android browser decode the path. This problem was actually meant to be addressed after the patch in the bug below landed:

https://bugs.webkit.org/show_bug.cgi?id=23546

but it never happened.

Also note the following bug filed against Android:

http://code.google.com/p/android/issues/detail?id=3861

Uploading patch soon.
------- Comment #1 From 2009-10-08 11:28:54 PST -------
Created an attachment (id=40895) [details]
Make KURL::prettyUrl (used by window.location.href) not URI-decode the URI path.
------- Comment #2 From 2009-10-08 11:31:02 PST -------
Created an attachment (id=40896) [details]
Make KURL::prettyUrl (used by window.location.href) not URI-decode the URI path.

Forgot to mark the previous upload as 'patch'.
------- Comment #3 From 2009-10-08 12:11:51 PST -------
(From update of attachment 40896 [details])
You ChangeLog is duplicated.

I don't really like pathRaw(), maybe encodedPath()?  Or should path be decodedPath()?

fast/dom/Window/Location/location-path-has-is-unmodified.htm is sorta a js test, except you didn't generate the tempate using make-script-test-wrappers.

Why does this need to be run from onload?  It needs to wait for the iframe to load?  Will body onload do that?

(If this needs to wait for onload, then it won't work quite right as a js test, so maybe that's why you ddid it this way.)

+        correctValue = "path%2Dwith-escape%2Dcharacters.html";
+        shouldBe("result", "correctValue");

shouldBeEqualToString("normalizeURL(String(window.frames[0].location.href))", "path%2Dwith-escape%2Dcharacters.html");
would have been better.

Does this actually need to load a valid resource to work?
------- Comment #4 From 2009-10-08 13:19:10 PST -------
This should not change the behavior of prettyUrl. You can add a new function with the desired behavior and possibly remove prettyUrl if no one uses it. It doesn't make sense to have a “pretty” URL function that does not decode the path!

You should look at what window.location.href needs and think about what the right name is for it.
------- Comment #5 From 2009-10-12 08:53:06 PST -------
(In reply to comment #3)
> (From update of attachment 40896 [details] [details])
> You ChangeLog is duplicated.
> 
> I don't really like pathRaw(), maybe encodedPath()?  Or should path be
> decodedPath()?
> 

Actually, I think that using the conversion to String operator is enough. This returns the string that was built in the KURL::parse() method.

> fast/dom/Window/Location/location-path-has-is-unmodified.htm is sorta a js
> test, except you didn't generate the tempate using make-script-test-wrappers.
> 
> Why does this need to be run from onload?  It needs to wait for the iframe to
> load?  Will body onload do that?
> 

Yes.

> (If this needs to wait for onload, then it won't work quite right as a js test,
> so maybe that's why you ddid it this way.)
>

That's right. In fact, I just used the pattern I found in the fast/dom/HTMLObjectElement/object-as-frame.html test that I had to modify. Note that that test also uses an onload handler to test the window.location.href property of an iframe.



> +        correctValue = "path%2Dwith-escape%2Dcharacters.html";
> +        shouldBe("result", "correctValue");
> 
> shouldBeEqualToString("normalizeURL(String(window.frames[0].location.href))",
> "path%2Dwith-escape%2Dcharacters.html");
> would have been better.
> 

Done.

> Does this actually need to load a valid resource to work?

Actually it doesn't if I use a data URL. Done.
------- Comment #6 From 2009-10-12 08:58:31 PST -------
Hi Darin,

(In reply to comment #4)
> This should not change the behavior of prettyUrl. You can add a new function
> with the desired behavior and possibly remove prettyUrl if no one uses it. It
> doesn't make sense to have a “pretty” URL function that does not decode the
> path!
> 


You're right, it would not be "pretty" anymore. prettyURL() is used by the Console so I cannot remove it.

> You should look at what window.location.href needs and think about what the
> right name is for it.

I looked and, if I am right, then the String conversion operator of KURL should be enough, so I do not need to add anything new. This operator calls KURL::string(), which just returns the string that was the output of the parse method. So simply removing the call to prettyURL in Location.cpp seems to do the job and all the layout tests pass.

Thanks,
Andrei
------- Comment #7 From 2009-10-12 09:13:39 PST -------
(In reply to comment #6)
> I looked and, if I am right, then the String conversion operator of KURL should
> be enough, so I do not need to add anything new. This operator calls
> KURL::string(), which just returns the string that was the output of the parse
> method. So simply removing the call to prettyURL in Location.cpp seems to do
> the job and all the layout tests pass.

Sounds good.

The way to prove this is with tests. Think about cases where you think this might fail, and write a new test or tests that covers as many cases as possible. At least one test for each distinct thing prettyURL used to do, and also tests based on guesses you might have about what could go wrong. And then make sure you run the tests in other browsers too.
------- Comment #8 From 2009-10-12 09:27:29 PST -------
(In reply to comment #7)
> (In reply to comment #6)
> > I looked and, if I am right, then the String conversion operator of KURL should
> > be enough, so I do not need to add anything new. This operator calls
> > KURL::string(), which just returns the string that was the output of the parse
> > method. So simply removing the call to prettyURL in Location.cpp seems to do
> > the job and all the layout tests pass.
> 
> Sounds good.
> 
> The way to prove this is with tests. Think about cases where you think this
> might fail, and write a new test or tests that covers as many cases as
> possible. At least one test for each distinct thing prettyURL used to do, and
> also tests based on guesses you might have about what could go wrong. And then
> make sure you run the tests in other browsers too.

I was just about to upload a new patch but I will hold off for now and add more tests. Thanks for the comments!

Andrei
------- Comment #9 From 2010-09-07 01:01:16 PST -------
Please fix this bug as it makes it impossible to distinguish URLs like /%25 and /%2525 in a cross-browser manner.
------- Comment #10 From 2010-09-07 14:29:18 PST -------
Adam, have you looked into this issue while planning your URL work?
------- Comment #11 From 2010-09-07 14:48:13 PST -------
(In reply to comment #10)
> Adam, have you looked into this issue while planning your URL work?

I haven't.  I've been studying the behavior of <a href>.  In general, it makes sense to canonicalize URLs before returning them to web content.  For example, returning % unescaped is clearly problematic because % is a control character in URLs.
------- Comment #12 From 2011-05-20 10:30:03 PST -------
For what it is worth HTMLAnchorElement href and other URL DOM bindings use KURL::string which seems to do the right thing. Changing Location::href to use KURL::string as well seems to solve this particular bug without having to update KURL.
------- Comment #13 From 2011-05-26 08:42:17 PST -------
Chiming in with another request to please fix this issue -- it has the side effect of effectively making the new pushState() API unusable in Safari. If your URL happens to have encoded data in it, like say, the name of a user-entered field -- it's impossible to distinguish their "/" in the title from a true "/', because the auto-decoding makes them appear the same to window.location.pathname.
------- Comment #14 From 2011-05-26 10:09:55 PST -------
Thanks.  I'll look at this in connection with the prettyURL audit.
------- Comment #15 From 2011-05-26 22:39:35 PST -------
My investigation in Bug 61201 seems to indicate that all callers of prettyURL are wrong.  I'm going to wrap that up first and then continue on to this bug.
------- Comment #16 From 2011-05-27 06:35:24 PST -------
Many thanks, Adam. -- I very much appreciate you taking a look at this issue.
------- Comment #17 From 2011-08-16 17:44:13 PST -------
Taking... My intention is to make Safari match the other browsers here
------- Comment #18 From 2011-08-17 15:42:06 PST -------
Created an attachment (id=104257) [details]
Patch
------- Comment #19 From 2011-08-17 15:48:53 PST -------
This makes Safari a whole lot more compatible with IE, Firefox and Opera which all return the encoded form for these things.

Sorry about the incomplete ChangeLog files, I forgot that webkit-patch does not pick up changes to these things.
------- Comment #20 From 2011-08-17 16:00:42 PST -------
(From update of attachment 104257 [details])
This patch is great.
------- Comment #21 From 2011-08-17 16:02:23 PST -------
 > Sorry about the incomplete ChangeLog files, I forgot that webkit-patch does not pick up changes to these things.

(I should also say that I'm assuming you wrote a nice ChangeLog entry that you'll include when landing the patch.)
------- Comment #22 From 2011-08-17 16:04:11 PST -------
(From update of attachment 104257 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=104257&action=review

> Source/WebCore/page/Location.cpp:68
> -    const KURL& url = this->url();
> -    // FIXME: Stop using deprecatedString(): https://bugs.webkit.org/show_bug.cgi?id=30225
> -    return url.hasPath() ? url.deprecatedString() : url.deprecatedString() + "/";
> +    return this->url().string();

I don’t see any coverage of "http://foo.com", which is a case that I’d expect to be affected by the hasPath check. Is that covered and I just missed it?

The old code also might do things like omit a "@" or a ":" where the new code would keep it. Is that covered?

> Source/WebCore/platform/KURL.cpp:604
> -    return decodeURLEscapeSequences(m_string.substring(start, m_hostEnd - start));
> +    return m_string.substring(start, m_hostEnd - start);

This change seems entirely separate from the Location.href/toString one. Is there sufficient test coverage? I don’t see tests where hostnames are seen URL-encoded above.

> Source/WebCore/platform/KURL.cpp:729
> -    return decodeURLEscapeSequences(m_string.substring(m_portEnd, m_pathEnd - m_portEnd)); 
> +    return m_string.substring(m_portEnd, m_pathEnd - m_portEnd); 

Are you sure that all callers of path want the URL-escaped form? For example, if I was going to take a file URL’s path, encode as UTF-8 and use it to access a file, I would not want it URL-escaped. Is there any code that does that?
------- Comment #23 From 2011-08-17 16:05:51 PST -------
(From update of attachment 104257 [details])
Attachment 104257 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9419246

New failing tests:
fast/dom/javascript-backslash.html
fast/dom/HTMLObjectElement/object-as-frame.html
------- Comment #24 From 2011-08-17 16:06:12 PST -------
(From update of attachment 104257 [details])
Looks like this will break DOMFileSystemBase::crackFileSystemURL
------- Comment #25 From 2011-08-17 16:07:08 PST -------
Fixing the location object seems fine, but the KURL changes are likely to have effects elsewhere. Please thoroughly investigate this before landing the change.
------- Comment #26 From 2011-08-17 16:07:44 PST -------
Looks like this will also break createGlobalHDropContent in ClipboardWin.cpp
------- Comment #27 From 2011-08-17 16:09:13 PST -------
Looks like this breaks KURL::fileSystemPath in KURLQt.cpp
------- Comment #28 From 2011-08-17 16:09:56 PST -------
This might break FrameLoader::defaultObjectContentType when the path extension has characters that are URL-encoded in it.
------- Comment #29 From 2011-08-17 16:10:21 PST -------
(From update of attachment 104257 [details])
Darin's comment make me less enthusiastic about this patch.  :)
------- Comment #30 From 2011-08-17 16:12:31 PST -------
(From update of attachment 104257 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=104257&action=review

>> Source/WebCore/page/Location.cpp:68
>> +    return this->url().string();
> 
> I don’t see any coverage of "http://foo.com", which is a case that I’d expect to be affected by the hasPath check. Is that covered and I just missed it?
> 
> The old code also might do things like omit a "@" or a ":" where the new code would keep it. Is that covered?

http://trac.webkit.org/browser/trunk/LayoutTests/fast/url/script-tests/standard-url.js is a good place to add this test.  It is covered there, but in an oblique way.  A direct test would be better.
------- Comment #31 From 2011-08-17 16:13:19 PST -------
Thanks Darin. I did expect this to need more work.

I'll look into these issues.

Another option is to introduce KURL::encodedPath() and KURL::encodedHost()
------- Comment #32 From 2011-08-17 18:21:38 PST -------
Darin, for the record Chromium has used encoded host() and path() for KURL since 1.0 without any currently known issues so I'm hoping I can get this to be the case for KURL in non Chromium code as well.
------- Comment #33 From 2011-08-17 18:31:55 PST -------
ClipboardWin.cpp and KURLQt.cpp aren't used by Chromium, so it's important to make sure we don't break these.  I'm less worried about FrameLoader::defaultObjectContentType, although it would be nice to have a test.

DOMFileSystemBase::crackFileSystemURL is used by Chromium, so if this change breaks that function, there must be something going on that I don't fully understand.
------- Comment #34 From 2011-08-18 10:32:07 PST -------
(In reply to comment #33)
> DOMFileSystemBase::crackFileSystemURL is used by Chromium, so if this change breaks that function, there must be something going on that I don't fully understand.

I believe this patch changes the behavior of that function so that file extensions that are percent-escaped are be handled differently from the same file extensions when not percent-escaped. I don’t know whether file extensions that are percent-escaped are legal or not.

So, for example if the extension was ".%6Apeg" instead of ".jpeg".
------- Comment #35 From 2011-08-24 13:56:43 PST -------
(In reply to comment #34)
> (In reply to comment #33)
> > DOMFileSystemBase::crackFileSystemURL is used by Chromium, so if this change breaks that function, there must be something going on that I don't fully understand.
> 
> I believe this patch changes the behavior of that function so that file extensions that are percent-escaped are be handled differently from the same file extensions when not percent-escaped. I don’t know whether file extensions that are percent-escaped are legal or not.
> 
> So, for example if the extension was ".%6Apeg" instead of ".jpeg".

The filesystem should support files named .%6Apeg and .jpeg, and should see them as distinct.  It currently has some bugs in the area, which I'm working on with https://bugs.webkit.org/show_bug.cgi?id=62811.

If you call toURL() on a FileEntry called ".%6Apeg", you should get "filesystem:http://your.origin.com/temporary/.%256Apeg".
------- Comment #36 From 2011-08-24 16:02:49 PST -------
My comment is not about whether the old or new behavior is correct. My comment is about the fact that this patch changes behavior, and is not fixing a bug with a test case.

I think a patch targeted at window.location should fix specifically window.location. Fixing the underlying function without testing all the call sites is unnecessarily dangerous.
------- Comment #37 From 2011-08-24 16:58:14 PST -------
Yes, definitely.  I asked EricU to look at this bug to help us understand what the correct behavior is and to see how it related to the other bug he's working on.
------- Comment #38 From 2011-08-24 17:10:28 PST -------
(In reply to comment #37)
> I asked EricU to look at this bug to help us understand what the correct behavior is and to see how it related to the other bug he's working on.

Thanks EricU. I appreciate having the information!
------- Comment #39 From 2011-09-23 17:41:30 PST -------
Created an attachment (id=108568) [details]
Patch
------- Comment #40 From 2011-09-23 17:44:25 PST -------
Sorry for not getting back to this sooner.

I ended up decoding the path in the places where it was expecting decoded values like before.

I only tested this on Safari/Mac so I expect some Chromium failures. I'll look at the bots when they come in.
------- Comment #41 From 2011-09-23 20:04:02 PST -------
(From update of attachment 108568 [details])
Attachment 108568 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9842119

New failing tests:
fast/loader/subframe-navigate-during-main-frame-load.html
fast/history/history-back-initial-vs-final-url.html
fast/url/standard-url.html
------- Comment #42 From 2011-10-04 15:59:01 PST -------
Created an attachment (id=109710) [details]
Patch
------- Comment #43 From 2011-10-04 16:01:27 PST -------
(In reply to comment #42)
> Created an attachment (id=109710) [details] [details]
> Patch

There are still KURL issues that this patch does not fix but (fragments encodes non ascii characters for example) but it does fix the path issue and cleans up the non hierarchical URLs too.
------- Comment #44 From 2011-10-04 16:30:06 PST -------
(From update of attachment 109710 [details])
Attachment 109710 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9958092

New failing tests:
fast/url/standard-url.html
------- Comment #45 From 2011-10-04 17:12:03 PST -------
Adam, the bot output does not include "standard-url" and it passing locally (built and tested with --chromium). Any advice?
------- Comment #46 From 2011-10-04 17:22:54 PST -------
You're probably testing on a Mac.  I suspect there are different expectations for standard URL on Windows and then Linux needs to further override the windows expectations.
------- Comment #47 From 2011-10-04 17:23:53 PST -------
Notice that the fallback path for chromium-linux goes through chromium-win:

https://docs.google.com/drawings/d/1z65SKkWrD4Slm6jobIphHwwRADyUtjOAxwGBVKBY8Kc/edit?hl=en_US
------- Comment #48 From 2011-10-04 17:28:46 PST -------
(From update of attachment 109710 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=109710&action=review

> LayoutTests/fast/dom/anchor-origin-expected.txt:12
> +data:text/html,foo => null

This shows the test is now broken. The URL has <b> tags in it, but now they disappear. We should fix the test so the <b> tags don’t disappear. I understand they should not be %-escaped, but it’s bad to leave this test in a broken state.

> LayoutTests/fast/url/script-tests/file-http-base.js:39
> -    ["file:///C:/asdf#\\xc2", "file:///C:/asdf#\\xef\\xbf\\xbd"],
> +    ["file:///C:/asdf#\xc2", "file:///C:/asdf#\xc2"],

I don’t understand why you changed what’s being tested here. Changing the result is one thing, but you changed the test input URL as well. Don’t we still want to test that case? Maybe we should test both?

> LayoutTests/fast/url/script-tests/standard-url.js:64
> +  ["javascript:alert(\\t 1 \\n\\r)", "javascript:alert( 1 )"],

Is this really an expected, good test result? Why are the spaces being collapsed here?

> LayoutTests/fast/url/segments.html:4
> -<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
> +<!DOCTYPE html>
>  <html>
>  <head>
> +<meta charset="utf-8">

Why this change? Change log doesn’t mention it.

> Source/WebCore/fileapi/DOMFileSystemBase.cpp:46
> +#include "KURL.h"

This file already uses KURL. No need to add a new include.

> Source/WebCore/loader/FrameLoader.cpp:75
> +#include "KURL.h"

This file already uses KURL. No need to add a new include.

> Source/WebCore/page/Location.cpp:154
>  String Location::toString() const
>  {
> -    if (!m_frame)
> -        return String();
> -
> -    const KURL& url = this->url();
> -    // FIXME: Stop using deprecatedString(): https://bugs.webkit.org/show_bug.cgi?id=30225
> -    return url.hasPath() ? url.deprecatedString() : url.deprecatedString() + "/";
> +    return href();
>  }

If this is just turning around and calling another function, then lets inline this in the header.

> Source/WebCore/platform/win/ClipboardWin.cpp:43
> +#include "KURL.h"

No need to add this include. The file already used KURL member functions so it must already have been including this.

> Source/WebCore/workers/WorkerLocation.cpp:80
>  String WorkerLocation::toString() const
>  {
> -    // FIXME: Stop using deprecatedString(): https://bugs.webkit.org/show_bug.cgi?id=30225
> -    return m_url.hasPath() ? m_url.deprecatedString() : m_url.deprecatedString() + "/";
> +    return href();
>  }

Same here.
------- Comment #49 From 2011-10-05 17:26:25 PST -------
(From update of attachment 109710 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=109710&action=review

>> LayoutTests/fast/dom/anchor-origin-expected.txt:12
>> +data:text/html,foo => null
> 
> This shows the test is now broken. The URL has <b> tags in it, but now they disappear. We should fix the test so the <b> tags don’t disappear. I understand they should not be %-escaped, but it’s bad to leave this test in a broken state.

Fixed.

>> LayoutTests/fast/url/script-tests/file-http-base.js:39
>> +    ["file:///C:/asdf#\xc2", "file:///C:/asdf#\xc2"],
> 
> I don’t understand why you changed what’s being tested here. Changing the result is one thing, but you changed the test input URL as well. Don’t we still want to test that case? Maybe we should test both?

Fixed. I added a new test line here too.

>> LayoutTests/fast/url/script-tests/standard-url.js:64
>> +  ["javascript:alert(\\t 1 \\n\\r)", "javascript:alert( 1 )"],
> 
> Is this really an expected, good test result? Why are the spaces being collapsed here?

This is how all other browsers behave.

>> LayoutTests/fast/url/segments.html:4
>> +<meta charset="utf-8">
> 
> Why this change? Change log doesn’t mention it.

The test script is now UTF-8. I added a comment to the ChangeLog.

>> Source/WebCore/page/Location.cpp:154
>>  }
> 
> If this is just turning around and calling another function, then lets inline this in the header.

Fixed.
------- Comment #50 From 2011-10-05 17:26:41 PST -------
Created an attachment (id=109887) [details]
Patch
------- Comment #51 From 2011-10-05 17:31:01 PST -------
(From update of attachment 109887 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=109887&action=review

> LayoutTests/ChangeLog:23
> +        * fast/url/segments.html: Use UT-8

Typo
------- Comment #52 From 2011-10-05 17:52:15 PST -------
Created an attachment (id=109895) [details]
Patch for landing
------- Comment #53 From 2011-10-05 18:39:14 PST -------
(From update of attachment 109895 [details])
Clearing flags on attachment: 109895

Committed r96779: <http://trac.webkit.org/changeset/96779>
------- Comment #54 From 2011-10-05 18:39:23 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #55 From 2011-10-05 19:22:56 PST -------
It seems like this patch caused fast/history/history-back-initial-vs-final-url.html to fail on Qt:
http://build.webkit.org/results/Qt%20Linux%20Release/r96779%20(38208)/results.html
------- Comment #56 From 2011-10-05 19:51:53 PST -------
(In reply to comment #55)
> It seems like this patch caused fast/history/history-back-initial-vs-final-url.html to fail on Qt:
> http://build.webkit.org/results/Qt%20Linux%20Release/r96779%20(38208)/results.html

http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp#L834

Needs to be changed to

QString url = item.url().toString();

since the extra encoding is no longer needed to match KURL.

I can make this change tomorrow when I get in to the office.
------- Comment #57 From 2011-10-06 03:48:21 PST -------
Fix landed in http://trac.webkit.org/changeset/96789.
But unfortunately there is one more bug: https://bugs.webkit.org/show_bug.cgi?id=69511
------- Comment #58 From 2011-10-06 13:22:20 PST -------
*** Bug 22899 has been marked as a duplicate of this bug. ***
------- Comment #59 From 2012-01-23 14:07:23 PST -------
*** Bug 76320 has been marked as a duplicate of this bug. ***
------- Comment #60 From 2012-03-28 15:46:39 PST -------
Has this fix made its way into Safari 5.1.5?

Using that version of Safari, if I go to this page:
http://limpet.net/mbrubeck/temp/chromium-location-test.html/foo%20bar?foo%20bar

The non-query part of the location is still being decoded.
------- Comment #61 From 2012-04-09 11:41:33 PST -------
*** Bug 83485 has been marked as a duplicate of this bug. ***