fix: multibase in pubsub http rpc#8183
Merged
lidel merged 17 commits intoipfs:masterfrom Nov 29, 2021
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This updates HTTP RPC wire format to wrap binary data in multibase to fix bug entire class of bugs described in #7939
TODO
\n)decide if we prefer to keep public API unchanged, or switch topic names to be[]bytedecide if we are ok with js-ipfs and js-ipfs-http-client returning peerid as CIDv1 while go-ipfs uses old notationpubsubtests in ipfs/interop need JS and GO to act the same, so we need to land:switching to npm version with fix: disable pubsub until http rpc changes land interop#388 so other PRs and master are not impacted.
merge fix: multibase in pubsub http rpc interop tests interop#387 after both GO and JS releases shipped and swich to new release
Closes #7939
Closes #8454
Click to expand original description from Cory
ipfs pubsub pubgets wrapped in multibasemultibase encoding does not remain throughout the entire system, only for publishing. My thinking is that in json responses, the data is already base64-encoded. multibase-encoded url-strings provides some benefit to producers, but there would be a size penalty for subscribers that have double-encoded. I could be convinced otherwise, though.