Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(jsonrpc): use padded-base64 for payload encoding #1572

Merged
merged 1 commit into from
Feb 23, 2023
Merged

Conversation

LNSD
Copy link
Contributor

@LNSD LNSD commented Feb 22, 2023

This PR includes code and API cleanup and adds tests covering the base64 module. A typical gardening PR 🪴

Note
The v0.15.0 is working as expected: Payload must be encoded using regular base64 (with padding).


Regarding the RFC discussion about the JSON-RPC message payload encoding using base64 (here):

Ok... There was some confusion on my side: I thought the payload was encoded in base64 URL-friendly format without padding. This is not what was happening. It is encoded in regular base64 with padding. It seems I implemented it the right way from the very beginning.

So the v0.15.0 is working as expected: Payload must be encoded using regular base64 (with padding).

I have updated this PR so it is clear that the encoding must be with padding.

@fryorcraken
Copy link
Collaborator

I can confirmed this fixes the issues previously reported in #1564
With this PR, nwaku know accepts NodeJS Buffer's base64 string encoding.

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@LNSD LNSD merged commit 8258415 into master Feb 23, 2023
@LNSD LNSD deleted the fix-jsonrpc-base64 branch February 23, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants