elixir-grpc / grpc

An Elixir implementation of gRPC
https://hex.pm/packages/grpc
Apache License 2.0
1.38k stars 212 forks source link

`grpc-message` is not percent-encoded as specified in the gRPC HTTP/2 spec #269

Closed clww closed 1 year ago

clww commented 2 years ago

Describe the bug

In gRPC over HTTP/2 spec, it says that the grpc-message should be percent-encoded.

When sending grpc-message containing unencoded char %, this will cause some gRPC clients (e.g. in my case, grpc-js) to throw an exception.

Even though the spec also says (the client) MUST NOT error or throw away the message, it's still suggested to add the encoding on the server side (I'm not sure whether there is a corresponding decoding part on the elixir-grpc client side). Meanwhile, we also created an issue in grpc-js and it has been addressed.

Expected behavior

grpc-message should be correctly encoded as the spec.

polvalente commented 2 years ago

@clww PRs are welcome! A possible solution is using Base.url_encode64 for serialization and then undo that during deserialization. Any thoughts?

clww commented 2 years ago

@polvalente I'd love to though I'm not a Elixir developer (I work on web front-end stuff), but I can ask my Elixir dev colleagues to take a look at this.

About Base.url_encode64 (I assume that this method encodes a string to a url-safe base64 string), I don't think it's the exact method we want to encode/decode the grpc-message here, as the spec says the string needs to be percent-encoded. I guess the decode/encode methods in https://hexdocs.pm/elixir/1.12/URI.html are more likely to be the ones.

polvalente commented 2 years ago

@polvalente I'd love to though I'm not a Elixir developer (I work on web front-end stuff), but I can ask my Elixir dev colleagues to take a look at this.

Sure! Let me know if they can't take this on and we can see if anyone else is able to tackle this.

About Base.url_encode64 (I assume that this method encodes a string to a url-safe base64 string), I don't think it's the exact method we want to encode/decode the grpc-message here, as the spec says the string needs to be percent-encoded. I guess the decode/encode methods in https://hexdocs.pm/elixir/1.12/URI.html are more likely to be the ones.

Yes, this encodes the text in a url-safe way, which means that nothing needs to be percent-encoded (https://hexdocs.pm/elixir/1.12/Base.html#module-base-64-url-and-filename-safe-alphabet). Downside for this one is that the transmitted payload increases to 4/3 of the original size (33% more).

URI encode/decode is probably the best solution. I had forgot about those functions. It seems that is handles non-string binaries just fine as well:

iex(16)> URI.encode("asdf" <> <<0>>) |> URI.decode
<<97, 115, 100, 102, 0>>
iex(17)> "asdf" <> <<0>>
<<97, 115, 100, 102, 0>>

edit: looking at the source-code for URI.encode, it will work with any binary, although multi-byte characters will have each byte encoded individually, but the decoding undoes that without hiccups

iex(18)> "josé" |> URI.encode()
"jos%C3%A9"
iex(19)> "josé" |> URI.encode() |> URI.decode()
"josé"