Server websocket implementation should allow, ignore other subprotocols

[niloc132]

Correctly reads all Sec-Websocket-Protocol headers and reads all values
in those headers, and only succeeds if the expected subprotcol value of
"grpc-websockets" is present. This implementation roughly mirrors what
the nhooyr.io/websocket Accept() method does, with a few
simplifications.

Fixes #1111

Changes

Updated IsGrpcWebSocketRequest() to allow for multiple protocol headers and values, but still only succeeds if the expected value grpc-websockets is present. Uses strings.EqualFold in place of strings.toLower and a == check, as this is a more flexible check, and reflects the underlying nhooyr.io/websockets implementation.

Verification

• Tested with a lightly modified websocket.ts transport, which advertises an alternative subprotocol.
• Tested with a multiplex-capable websocket transport, which can fall back to the existing websocket transport in case the server doesn't also support multiplex.

[johanbrandhorst]

One very minor nit, this looks great.

[niloc132]

The "test failure" looks like it might be a transient saucelabs tunnel issue?

https://app.circleci.com/pipelines/github/improbable-eng/grpc-web/665/workflows/1ccb9f11-3ca4-403e-b584-40379472b9de/jobs/6077

...
+ sleep 1
+ test 2 -eq 0 -o -e ./sauce-connect-readyfile-no-ssl-bump
+ sleep 1
+ test 1 -eq 0 -o -e ./sauce-connect-readyfile-no-ssl-bump
+ sleep 1
+ test 0 -eq 0 -o -e ./sauce-connect-readyfile-no-ssl-bump
+ (( ++wait_seconds ))
+ echo 'Timed out waiting for sauce labs tunnel (with ssl bump)'
+ kill 1369
./run-with-tunnel.sh: line 93: kill: (1369) - No such process
+ killTunnels
+ echo 'Killing Sauce Labs Tunnels...'
62.085767+ [[ -z '' ]]
 Timed out waiting for sauce labs tunnel (with ssl bump)
+ kill 1369
./run-with-tunnel.sh: line 79: kill: (1369) - No such process
62.085857 Killing Sauce Labs Tunnels...
npm ERR! code 1
npm ERR! path /go/src/github.com/improbable-eng/grpc-web/integration_test
npm ERR! command failed
npm ERR! command sh -c ./run-with-tunnel.sh ./run-with-testserver.sh karma start ./karma.conf.ts --single-run
...

Can someone re-run that?

[johanbrandhorst]

Thank you for your contribution!

[niloc132]

@johanbrandhorst would it be reasonable to request a release be cut, for this and a few other fixes made since the release last November?

[johanbrandhorst]

That sounds reasonable :). The release process isn't super automated, so I'm gonna have to ask @MarcusLongmuir for some help.

[johanbrandhorst]

Ping @MarcusLongmuir or @easyCZ, either of you have bandwidth for a new release?