-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
WebSocket handshake has no referrer #8280
Conversation
Build PASSEDStarted: 2017-11-17 12:38:52 View more information about this build on: |
ws.onmessage = t.step_func_done(e => { | ||
assert_equals(e.data, "MISSING AS PER FETCH"); | ||
ws.close(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do this to avoid timeouts
ws.onclose = t.unreached_func('close');
ws.onerror = t.unreached_func('error');
Locally I get an error event with chrome canary running referrer.any.html. I haven't debugged why that is. Can you reproduce though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot, Chrome 64.0.3271.0 on macOS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see what I did wrong. I didn't restart wptserve after checking out your branch, so the websocket handler /referrer
didn't exist. Sorry. Now it passes for me locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, took me a while to figure that one out too.
websockets/handlers/referrer_wsh.py
Outdated
|
||
def web_socket_transfer_data(request): | ||
referrer = request.headers_in.get("Referer") | ||
if not referrer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be if referrer is None
, at least other handlers use that form and I vaguely recall changing to more explicit None
checks for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
See whatwg/fetch#632.