User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.95 Safari/537.36 Steps to reproduce: 1. Prepare a script like this. //request_check.php <?php exit(var_dump($_SERVER['REQUEST_URI'])); ?> 2. Access the script with query parameter. http://localhost/request_check.php?hoge=fuga&hoge=piyo 3. You will get a string like this. IE8 string(51) "/otsukano/request_check.php?hoge=fuga&hoge=piyo" Chrome string(51) "/otsukano/request_check.php?hoge=fuga&hoge=piyo" FireFox35 string(59) "/otsukano/request_check.php?hoge%5B%5D=fuga&hoge%5B%5D=piyo" Actual results: The value of $_SERVER['REQUEST_URI']) is url-encoded unexpectedly ( => %5B%5D). Expected results: The value should not be url-encoded.
Summary: Some charactors([, ]) are url encoded unexpectedly in $_SERVER variable. → Some charactors([, ]) in URL are url-encoded unexpectedly in $_SERVER variable.
I tried to find the relevant spec bits for this. Reading https://url.spec.whatwg.org/#url-writing , it seems it says that a query string must consist of URL units, which are either percent-encoded bytes or in the set of url code points: https://url.spec.whatwg.org/#url-code-points which don't include [ or ] , leading me to think that they should therefore be encoded. However, (a) my success rate when trying to read specs is not great, and (b) the cross-browser inconsistency is not a good sign here. Anne/bz/valentin, you probably know more than me, can (one of) you comment?
Component: Untriaged → Untriaged
OS: Windows 7 → All
Product: Firefox → Core
Hardware: x86_64 → All
That change was made in bug 473822. :annevk would know which is the right thing to do here.
Per https://url.spec.whatwg.org/#query-state it appears these are not to be escaped when found in a URL. It seems they are non-conforming to appear as such (they are not URL code points), but at the moment they do not get escaped by the parser and therefore what I suggested in bug 473822 is wrong. Testing other browsers using http://intertwingly.net/projects/pegurl/liveview.html shows that none of them escape either "[" or "]", but that does not explain the behavior people are talking about in the other bug. Perhaps the only problem was that we removed percent-encoding rather than leaving it in place? I'm really sorry about not having done due diligence the last time.
Status: UNCONFIRMED → NEW
Component: Untriaged → Networking
Ever confirmed: true
> my success rate when trying to read specs is not great Nor is mine :-) One thing I'm much better at is looking at actual test results: https://url.spec.whatwg.org/interop/test-results/ None of these tests include square brackets outside of the authority/hostname field. So lets add some. If people have suggestions for a few tests, I can add the tests, gather results, and we can discuss the way forward. Meahwhile, the following may be helpful: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3368
Summary: Some charactors([, ]) in URL are url-encoded unexpectedly in $_SERVER variable. → Some characters([, ]) in URL are url-encoded unexpectedly in $_SERVER variable.
The relevant spec is RFC 3986. "pchar" does not allow "[" nor "]", thus Firefox is correct in percent-encoding these characters. A recipient that trips over percent-encoding like this is broken and should be fixed.
(In reply to Anne (:annevk) from comment #3) > Testing other browsers using > http://intertwingly.net/projects/pegurl/liveview.html shows that none of > them escape either "[" or "]", but that does not explain the behavior people > are talking about in the other bug. Just to clarify about bug 473822, the behavior that was causing issues for my team was Firefox decoding correctly percent-encoded square brackets in the URL bar, and making that "sticky". Typing in "?foo%5Bb%5D=bar" correctly sent that to the server, but in the URL bar it displayed "?foo[b]=bar". Now if you copied/pasted, bookmarked, or even clicked in to the URL bar and pressed enter, the next request to the server used the naked square brackets. It was this behind-the-scenes changing of what the user had entered (or been directed to) that was causing issues.
Thanks Anthony, I agree that we should stop doing that. (I suppose that for display purposes we still might want to show [ and ], but we should not give an altered URL when you start to manipulate or copy it. Same as with Unicode <> ASCII.)
Created attachment 8549944 [details] [diff] [review] backout cc192030c28f - brackets shouldn't be automatically escaped in the Query I reverted the change in nsEscape.cpp and changed the unit test. This should probably be uplifted so users don't get stuck with the bug for too long
Attachment #8549944 - Flags: review?(mcmanus)
Assignee: nobody → valentin.gosu
Status: NEW → ASSIGNED
Attachment #8549944 - Flags: review?(mcmanus) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Comment on attachment 8549944 [details] [diff] [review] backout cc192030c28f - brackets shouldn't be automatically escaped in the Query Approval Request Comment [Feature/regressing bug #]: Bug 473822 [User impact if declined]: Some websites may not work for users on those specific releases. [Describe test coverage new/current, TBPL]: I have changed a test to make sure this behaviour does not regress in the future. [Risks and why]: Very low risk. It reverts back to a behaviour Firefox has had for a long time. [String/UUID change made/needed]: None
status-firefox36: --- → affected
status-firefox37: --- → fixed
status-firefox38: --- → fixed
status-firefox36: affected → fixed
status-firefox37: affected → fixed
This seems to have a dedicated straight forward test. Sylvestre, do you think it needs some additional manual testing outside of what's covered automatically?
Actually, no. Sorry about the noise!
You need to log in before you can comment on or make changes to this bug.