Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update WebSocket construction per mixed content #222

Closed
wants to merge 1 commit into from

Conversation

domenic
Copy link
Member

@domenic domenic commented Oct 1, 2015

Per https://w3c.github.io/webappsec/specs/mixedcontent/#websockets-integration, the preferred behavior for mixed content web sockets is not to throw a SecurityError during construction, but instead block the connection, just like with other network APIs.

Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=28841.

@mikewest, saw this lonely bug over there and thought I'd fix it for ya.

Per https://w3c.github.io/webappsec/specs/mixedcontent/#websockets-integration, the preferred behavior for mixed content web sockets is not to throw a SecurityError during construction, but instead block the connection, just like with other network APIs.

Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=28841.
@domenic domenic mentioned this pull request Oct 1, 2015
2 tasks
@mikewest
Copy link
Member

mikewest commented Oct 2, 2015

LGTM, though until the protocol bits are updated with http://www.rfc-editor.org/errata_search.php?rfc=6455&eid=4398, this leaves things fairly broken. I think @annevk was looking into bringing that into Fetch? :)

@annevk
Copy link
Member

annevk commented Oct 2, 2015

No we should not merge this. See #180. We should make a plan first.

@zcorpan zcorpan added the do not merge yet Pull request must not be merged per rationale in comment label Oct 2, 2015
@annevk
Copy link
Member

annevk commented Jan 4, 2016

I tried to figure out a plan, but it's unclear to me what is best. Clearly it would be better if some of the API-level requirements became network-requirements instead, but currently we have no control over the network. Perhaps maintaining a (big) monkey patch in HTML is the way to go? Figuring out if we can reuse Fetch in some fashion also still seems attractive, since duplicating all the requirements is kind of cumbersome.

@domenic
Copy link
Member Author

domenic commented Feb 24, 2016

@annevk I'd like to either merge this or close the PR and re-open the bug. To me it seems like an improvement over the status quo, but maybe not. Happy to go with whichever you decide.

@annevk
Copy link
Member

annevk commented Feb 26, 2016

I think doing this in isolation doesn't make the WebSocket situation clearer, since we have landed HSTS earlier.

We should first decide where they happen API or network (probably most of them network) and then write a monkey patch on top of the crappy RFC. Maybe Fetch should provide all the relevant WebSocket network hooks and then Fetch can handle all the necessary monkey patching.

Would love to know what @mikewest thinks if he can still remember.

@domenic
Copy link
Member Author

domenic commented Feb 26, 2016

OK. I will close this and re-open the bug then.

@domenic domenic closed this Feb 26, 2016
@domenic domenic deleted the websocketmixed branch March 1, 2016 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge yet Pull request must not be merged per rationale in comment
Development

Successfully merging this pull request may close these issues.

4 participants