Page MenuHomePhabricator

Invalid location header per RFC 7231 in Chrome: Redirects have path percent-escaped, but not fragment ("title contains invalid characters" errors)
Closed, DuplicatePublic

Description

Recently at ru.wikipedia.ru Location headers started to be served uncorrectly (partially encoded, partially not). While some browsers have a fix for it, latest Chrome (most importantly) and some other branches of WebKit was not ready. As a result after each edit redirects lead to "type 25 pollution" of URLs and to "unacceptable symbol %D0" error page. Like this one (see parasite 25's in the URL)

https://ru.wikipedia.org/wiki/%25D0%2592%25D0%25B8%25D0%25BA%25D0%25B8%25D0%25BF%25D0%25B5%25D0%25B4%25D0%25B8%25D1%258F:%25D0%25A4%25D0%25BE%25D1%2580%25D1%2583%25D0%25BC/%25D0%25A2%25D0%25B5%25D1%2585%25D0%25BD%25D0%25B8%25D1%2587%25D0%25B5%25D1%2581%25D0%25BA%25D0%25B8%25D0%25B9#%D0%9D%D0%B5_%D0%BA%D0%BE%D1%80%D1%80%D0%B5%D0%BA%D1%82%D0%BD%D0%BE%D0%B5_%D1%81%D0%BE%D1%85%D1%80%D0%B0%D0%BD%D0%B5%D0%BD%D0%B8%D0%B5_%D1%81%D1%82%D1%80%D0%B0%D0%BD%D0%B8%D1%86%D1%8B

The problem is reported to Chromium and they will restore the fix (their todo letter enclosed at the bottom). Still they asked to mention, that it is not a correction but a fix to fight with some servers non-standard behavior - ru.wikipedia.org this time.

So I'm mentioning it here. And if the headers will be corrected right now, Chrome users will not have to wait for the fix to have their work at Wikipedia restored.

The Chromium letter:

Fix handling of half-escaped Location headers.

Non-ASCII characters in URLs are supposed to be percent-escaped. This
includes when those characters are used in Location headers. However, we
tolerate unescaped characters by escaping them ourselves at various
stages in the process.

ru.wikipedia.org sometimes sends redirects where the path is
percent-escaped, but the fragment is not. Previously, we would fix the
escaping in the fragment, but leave the path as-is. However,
https://chromium-review.googlesource.com/c/chromium/src/+/1366759
switched it to always unformly escape everything, so the %s became %25.
This causes us to follow the redirect incorrectly.

Uniformly escaping things makes sense for NetLog, where we want to
unambiguously represent what the server actually sent, but escaping as
part of URL resolution as not intended to be invertible. Restore the old
behavior here and add some regression tests.

Bug: 942073
Change-Id: I009bc0dc9c5c8f836f072fe23ccd824698d550e0

Event Timeline

Aklapper renamed this task from Location header is invalid per RFC 7231: breaking Chrome browser usage to Invalid location header per RFC 7231 in Chrome: Redirects have path percent-escaped, but not fragment ("title contains invalid characters" errors).Mar 15 2019, 11:35 AM
matmarex added subscribers: MaxSem, matmarex.

Here's an example of the weirdly encoded header (sent when saving a null edit of the section on https://ru.wikipedia.org/wiki/Олсен,_Джастин)

Location: https://ru.wikipedia.org/w/index.php?title=%D0%9E%D0%BB%D1%81%D0%B5%D0%BD,_%D0%94%D0%B6%D0%B0%D1%81%D1%82%D0%B8%D0%BD&stable=0#Ссылки

I'm not very familiar with the relevant code (I'm here because I just saw the other task being merged as a duplicate on IRC and was curious), but I know that @MaxSem worked on this feature (human-readable section IDs).

Possibly Sanitizer::escapeIdForLink() is where the URL-encoding should be added?

I'm honnestly puzzled why does it take so long to fix.

Sending Locations of the kind %D0%A3%D1%87%D0%B0%D1%81%D1%82%D0%BD%D0%B8%D0%BA:Neolexx#Якорь (half encoded, half as-is Cyrillic) - is just plain wrong, logically and by RFC. Yes, the majority of browsers are able to fight with it (and many other things within IDN homograph attacks). But here is not a hacker site.

Chrome is a widely used browser, ru-wiki is one of biggest projects, this kind of "type 25 pollution" fix is plain ridiculous bu tonly one working so far: https://ru.wikipedia.org/wiki/User:Neolexx/common.js

So first fixing the Location headers and after starting to think what nice but not survival important things may got broken.

@Neolexx: This task is closed as a duplicate. See the other task for updates: A patch has been proposed only 36 hours ago. That's not "so long". If you'd like to get things fixed faster and want to help, feel free to amend the proposed patch to make it pass jenkins-bot verification.