Bug 157157 – Allow non-standard HTTP headers in WebSocket handshake

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[John Wilander]

Several reports of regressions in the wild mean we have to relax our header policy in the WebSocket handshake.

[John Wilander]

Created attachment 277666 [details]
Patch

[Brent Fulgham]

Comment on attachment 277666 [details]
Patch

r=me.

[WebKit Commit Bot]

Comment on attachment 277666 [details]
Patch

Clearing flags on attachment: 277666

Committed r200219: <http://trac.webkit.org/changeset/200219>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

Comment on attachment 277666 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277666&action=review

> Source/WebCore/ChangeLog:9
> +        No new tests since https://bugs.webkit.org/show_bug.cgi?id=157095
> +        tests that non-standard headers are allowed.

Was there a concrete motivation for further behavior change? If that's the case, then we should have a test that verifies that the specific new case keeps working.

Even if not, every behavior change should have a test, so adding a test for e.g. "Foo: bar" would be appropriate.

[Brent Fulgham]

Comment on attachment 277666 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277666&action=review

>> Source/WebCore/ChangeLog:9
>> +        tests that non-standard headers are allowed.
> 
> Was there a concrete motivation for further behavior change? If that's the case, then we should have a test that verifies that the specific new case keeps working.
> 
> Even if not, every behavior change should have a test, so adding a test for e.g. "Foo: bar" would be appropriate.

He already created tests for the issues we knew about (Sec-WebSocket-Location, WebSocket-location, etc.), but we have since found more instances of "custom" headers being used by developers.

I think it's fine to remove the tighter control on the headers, but retain the new "non-standard" tests that he added as a check that we don't start blocking them again.

I don't think further tests are warranted for this specific change.

[Alexey Proskuryakov]

If our reading of the spec is that any headers are allowed, it's still somewhat valuable to test for headers that have never been called out in any version of the spec.