Add support for gRPC-Web text encoding #150
Conversation
This adds support for gRPC-Web's text encoding. Only client support is added. Encoding of request and response body is handled to support text streams. Signed-off-by: Edward McFarlane <[email protected]>
Signed-off-by: Edward McFarlane <[email protected]>
Signed-off-by: Edward McFarlane <[email protected]>
emcfarlane
force-pushed
the
ed/grpcWebText
branch
from
25443cf to
1daeebb
Compare
jhump
left a comment
jhump
left a comment
There was a problem hiding this comment.
See comment. I think we could reimplement this as separate, dumber middleware that gets used from a Transcoder. WDYT?
| // ProtocolGRPCWebText is a variant of ProtocolGRPCWeb that uses base64 | ||
| // text encoding for the request and response bodies. | ||
| // | ||
| // This protocol is not supported on the server side. |
There was a problem hiding this comment.
Do we even need/want an exported constant for it?
It would likely be trivial to implement as a separate, simpler middleware handler. Then we have the option to potentially move it to connect-go in the future, to enable grpc-web-text there. The Transcoder could always delegate to that middleware first since it should basically be free/pass-through for non-grpc-web-text requests. Then we don't need a protocol constant for it at all since the rest of the logic in here doesn't need to know about it.
There was a problem hiding this comment.
It would also be relatively easy to test that separate middleware by copying it over into the connect conformance referenceserver and then see if the JS grpc-web-client starts passing the server stream tests that it currently opts out of. We'd have to enable grpc-web-text in that client, but that's as simple as changing the mode to "grpcwebtext" and re-generating.
(Admittedly, that's not a way to unit test this repo's implementation of it. But the tests you've added here could still be used of course.)
| } | ||
| } | ||
|
|
||
| func Test_grpcWebText(t *testing.T) { |
There was a problem hiding this comment.
| func Test_grpcWebText(t *testing.T) { | |
| func TestTranscoder_GrpcWebText(t *testing.T) { |
(Aside: at some point we should change the other battery of tests from TestMux_... to TestTranscoder_..., to reflect the actual API names.)
| // ProtocolGRPCWebText is a variant of ProtocolGRPCWeb that uses base64 | ||
| // text encoding for the request and response bodies. | ||
| // | ||
| // This protocol is not supported on the server side. |
There was a problem hiding this comment.
It would also be relatively easy to test that separate middleware by copying it over into the connect conformance referenceserver and then see if the JS grpc-web-client starts passing the server stream tests that it currently opts out of. We'd have to enable grpc-web-text in that client, but that's as simple as changing the mode to "grpcwebtext" and re-generating.
(Admittedly, that's not a way to unit test this repo's implementation of it. But the tests you've added here could still be used of course.)
This adds support for gRPC-Web's text encoding to handle
application/grpc-web-textcontent types. Only client support is added for now, for simplicity. Encoding of request and response body is handled to support text streams. Testing is added by mutating a gRPC-Web client to implement the text protocol.The implementation is done by converting the request body and response writer to encode and decode streams of base64 encoded data. Go's std library cannot handle padding characters within the stream. For writing base64 encoded data we can simply use
base64.NewEncoder, callingClosewhen needed to flush data to the writer. To read base64 encoded data we cannot usebase64.NewDecoderas we need to split the reader on padding characters to form valid base64 chunks to pass to the decoder. Therefore this implementation buffers the stream into 4-byte tokens before decoding, splitting on padding tokens when found.Closes #143