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

Reject websocket protocol requests that don't match https://tools.iet… #6694

Merged
merged 1 commit into from Jul 22, 2015

Conversation

@jdm
Copy link
Member

jdm commented Jul 22, 2015

…f.org/html/rfc6455#section-4.1 .

Review on Reviewable

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jul 22, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5621

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@jdm
Copy link
Member Author

jdm commented Jul 22, 2015

r? @Ms2ger

@jdm
Copy link
Member Author

jdm commented Jul 22, 2015

There are some more test expectation updates incoming, so hang tight.

@jdm jdm force-pushed the jdm:websocketprotocol branch from d7ac2e3 to 05beb99 Jul 22, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 22, 2015

-S-awaiting-review +S-needs-code-changes


Reviewed 6 of 7 files at r1.
Review status: 6 of 7 files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.


components/script/dom/webidls/WebSocket.webidl, line 7 [r1] (raw file):
File a followup for the array form


components/script/dom/websocket.rs, line 109 [r1] (raw file):
No need to allocate here. as_slice() would work here, or inline it if you want to avoid its instability.


components/script/dom/websocket.rs, line 121 [r1] (raw file):
protocols[i+1..].iter(); any()


components/script/dom/websocket.rs, line 125 [r1] (raw file):
any()


Comments from the review on Reviewable.io

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 22, 2015

Reviewed 1 of 7 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.


Comments from the review on Reviewable.io

@Ms2ger Ms2ger self-assigned this Jul 22, 2015
@jdm jdm force-pushed the jdm:websocketprotocol branch from 05beb99 to d385cb7 Jul 22, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 22, 2015

-S-awaiting-review +S-awaiting-merge

@bors-servo r+


Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Jul 22, 2015

📌 Commit d385cb7 has been approved by Ms2ger

@bors-servo
Copy link
Contributor

bors-servo commented Jul 22, 2015

Testing commit d385cb7 with merge 6b4f1a4...

bors-servo pushed a commit that referenced this pull request Jul 22, 2015
Reject websocket protocol requests that don't match https://tools.iet…

…f.org/html/rfc6455#section-4.1 .

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6694)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 22, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

@bors-servo bors-servo merged commit d385cb7 into servo:master Jul 22, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@jdm jdm deleted the jdm:websocketprotocol branch Aug 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.