Properly encode Uint8Array fields

[Unix4ever]

Fixes: #22

Binary data should be represented as base64 encoded string in the JSON
encoded body.
Use replacer to return base64 encoded string when Uint8Array type is
detected.

Signed-off-by: Artem Chernyshev artem.0xD2@gmail.com

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[Unix4ever]

@googlebot I signed it!

[lyonlai]

I like the replacer idea. It helps base64 encoded the uint8array before transfer. It does cover the data going up to the server, however I believe the data coming back down from the server are also base64 encoded. It would be nice to have the other side covered too.

For this case I would like to see an integration test to ensure the encoding/decoding logic. Could you please add a test into the integration_test folder? A simple bytes echo case should suffice. There's a README.md file in the folder to get you started.

Again, your contribution is much appreciated.

[lyonlai]

Sorry for the late reply as well.

[Unix4ever]

For this case I would like to see an integration test to ensure the encoding/decoding logic. Could you please add a test into the integration_test folder? A simple bytes echo case should suffice. There's a README.md file in the folder to get you started.

Yep, sure.

[Unix4ever]

I've added the tests, sorry for the delay.

Regarding the server -> client bytes flow: I've tried to get it done, but couldn't get it right due to my limited TypeScript knowledge.
While it's straightforward how to check the field type and see if it's Uint8Array and encode it to base64, it's not that clear how to detect base64 encoded string and convert it back to Uint8Array, given that it's just a simple string.

I was looking at how you can get type fields at the compile time and got lost in typeof keyof.
Another option is to generate object factories for each type and then run something like Factory<O>(r.json()) in fetchReq.

So I won't make this change in this PR. At least server -> client bytes flow works somehow (if you use any as a response receiver).

[Unix4ever]

Ugh, well. I was testing that more in my code and looks like btoa doesn't work for all cases. If the string does not consist only of Latin1symbols it'll throw the error.
I switched it to protobufjs/minimal util.base64.encode function in my code with the workaround, but I'm not sure how to fix it here.
Maybe embed proper base64 encoder right into the fetch module...

[lyonlai]

I see where you could stuck when the information coming down from the server, it's hard to tell as you said it's a string. And even if you use some test library to test if it was a base64 encoded string, you couldn't tell whether the field was an uint8array or not.

In this case you'll be looking for the compiler to generate some information to feed in the fetch call so that when the response comes back you can tell which field needs to be properly translate back to uint8array.

Doing just on the way up it is fine for this PR but if the problem on the way down is not properly solved it might need the user to do some type casting or ts-ignore as the typescript is expecting the field to be an uint8array, which is not that nice.

[lyonlai]

btoa has problem with non latin characters seems to be a common issue on the internet. protobufjs' base64 doesn't seems to be too big to embed in.

https://github.com/protobufjs/protobuf.js/blob/master/lib/base64/index.js#L42

[lyonlai]

LGTM, thank you for your contribution

[lyonlai]

I won't be making a release just yet until the receiver side's fixed. if you are interested to pick it up, let me know.

[Unix4ever]

I have plenty of other stuff on my plate already. The fix for the receiver side doesn't look simple while using typescript types. I tried to read more and check how it can be done but I've run out of ideas. 🤔