Skip to content

Conversation

@sten0
Copy link
Contributor

@sten0 sten0 commented Jul 25, 2020

Continuing from #64

What I had in mind was something like this PR. Unfortunately it doesn't yet work correctly, and seems to have exposed an unhandled failure case:

   passed  27/32  websocket-server-close
Invalid client headers found in: 


   passed  28/32  websocket-server-filter
   passed  29/32  websocket-to-bytes
Websocket client connection: Host header not found
Websocket client connection: Upgrade: websocket not found
Websocket client connect: No key sent
Websocket client connect: No key sent
Websocket client connection: HTTP/1.1 not found
   passed  30/32  websocket-verify-client-headers

Anyways, I thought I'd take a quick stab at the problem today, and this is what I came up with. Sorry if there are any dumb errors, I will confess it's rushed work.

@ahyatt
Copy link
Owner

ahyatt commented Jul 27, 2020

Thanks for the change. I should have time this weekend to take a close look at it.

@sten0
Copy link
Contributor Author

sten0 commented Jul 28, 2020 via email

@ahyatt
Copy link
Owner

ahyatt commented Jul 28, 2020

BTW, do you have FSF papers signed? I will need them in order to merge any significant change.

@sten0
Copy link
Contributor Author

sten0 commented Jul 28, 2020 via email

@ahyatt
Copy link
Owner

ahyatt commented Aug 4, 2020

Generally, this looks good - I agree, it doesn't look like more than 10 lines of actual changed code. I'll work on completing this shortly.

@ahyatt
Copy link
Owner

ahyatt commented Aug 4, 2020

I've put this into a branch on my side, and added commit 5aaf9d1, which makes everything work in a more or less seamless fashion. Let me know if you have any thoughts, if not, I'll merge this into the main branch.

@sten0
Copy link
Contributor Author

sten0 commented Aug 5, 2020 via email

@ahyatt ahyatt merged commit df976e3 into ahyatt:main Aug 6, 2020
@ahyatt
Copy link
Owner

ahyatt commented Aug 6, 2020

I've merged in your changes, plus my commit. Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants