Skip to content

Fix websocket-upgrade-request? for Firefox WebSocket upgrade requests. #562

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.

Sign up for GitHub

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

Jump to bottom
Merged
slipset merged 2 commits into from
Jan 15, 2021
Merged

Fix websocket-upgrade-request? for Firefox WebSocket upgrade requests. #562

slipset merged 2 commits into from
Jan 15, 2021

Conversation

@xorgy
Copy link
Contributor

@xorgy xorgy commented Jan 13, 2021

Fixes #561

@xorgy xorgy requested a review from slipset as a code owner January 13, 2021 16:18
@slipset
Copy link
Member

slipset commented Jan 13, 2021

Thanks a lot xorgy. Could you please also provide a test which shows that this doesn't break existing functionality, ie that it works when only sending "Upgrade" in the connection header, as well as when sending "keep-alive,Upgrade"?

@xorgy
Copy link
Contributor Author

xorgy commented Jan 13, 2021 •
edited
Loading

Thanks a lot xorgy. Could you please also provide a test which shows that this doesn't break existing functionality, ie that it works when only sending "Upgrade" in the connection header, as well as when sending "keep-alive,Upgrade"?

Sure, I'll write a test for it. Probably good to have one anyway, given that there are zero tests for this function right now.

@xorgy
Copy link
Contributor Author

xorgy commented Jan 13, 2021

Let me know if the test I've added is satisfactory, I can squash it in.

slipset
slipset approved these changes Jan 13, 2021
View reviewed changes
Copy link
Member

@slipset slipset left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

No need to squash, I'll do a squash merge here. Thanks!

@xorgy
Copy link
Contributor Author

xorgy commented Jan 13, 2021

Thank you as well.

@slipset slipset merged commit a704e00 into clj-commons:master Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slipset slipset slipset approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

websocket-upgrade-request? doesn't detect connection upgrade when there are other tokens

2 participants

@xorgy @slipset