Skip to content

Commit

Permalink
fix: multibase in pubsub http rpc (#8183)
Browse files Browse the repository at this point in the history
* multibase encoding on pubsub
* emit multibase for json clients
* refactor(pubsub): base64url for all URL args

This makes it easier to reason about.
Also added better helptext to each command explaining how the binary
data is encoded on the wire, and how to process it in userland.

* refactor: remove ndpayload and lenpayload

Those output formats are undocumented and seem to be only used in tests.
This change removes their implementation and replaces it with error
message to use JSON instead.

I also refactored tests to test the --enc=json response format instead
of imaginary one, making tests more useful as they also act as
regression tests for HTTP RPC.

* test(pubsub): go-ipfs-api

Testing against compatible version from
ipfs/go-ipfs-api#255

* refactor: safeTextListEncoder

Making it clear what it does and why

* refactor(pubsub): unify peerids

This ensures `ipfs pubsub sub` returns the same peerids in the `From`
field as `ipfs pubsub peers`.

libp2p already uses base encoding, no need to double wrap or use custom
multibase.

* test(pubsub): go-ipfs-http-client

* refactor(pubsub): make pub command read from a file

We want to send payload in the body as multipart so users can use
existing tools like curl for publishing arbitrary bytes to a topic.

StringArg was created for "one message per line" use case, and if data
has `\n` or `\r\n` byte sequences, it will cause payload to be split. It
is not possible to undo this, because mentioned sequences are lost, so
we are not able to tell if it was `\n` or `\r\n`

We already avoid this problem in `block put` and `dht put` by reading
payload via FileArg which does not mangle binary data and send it as-is.
It feel like `pubsub pub` should be using it in the first place anyway,
so this commit replaces StringArg with FileArg.

This also closes #8454
and makes rpc in go-ipfs easier to code against.

* test(pubsub): publishing with line breaks

Making sure we don't see regressions in the future.
Ref. #7939

* chore: disable pubsub interop for now

See
ipfs/interop@344f692

* test: t0322-pubsub-http-rpc.sh

- Adds HTTP RPC regression test that ensures topic is encoded as URL-safe
  multibase.
- Moves pubsub tests to live in unique range ./t032x

* fix(ci):  js-ipfs with  fixed pubsub wire format

uses js-ipfs from ipfs/js-ipfs#3922 
until js-ipfs release can ship with dependency on go-ipfs 0.11.0-rc1

Co-authored-by: Marcin Rataj <[email protected]>
Co-authored-by: Adin Schmahmann <[email protected]>
  • Loading branch information
3 people authored Nov 29, 2021
1 parent 72656ea commit a43e506
Show file tree
Hide file tree
Showing 6 changed files with 254 additions and 109 deletions.
12 changes: 12 additions & 0 deletions .circleci/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,15 @@ jobs:
- run:
name: Running tests
command: |
WORKDIR=$(pwd)
cd /tmp
git clone https://github.com/ipfs/js-ipfs.git
cd js-ipfs
git checkout 1dcac76f56972fc3519526e93567e39d685033dd
npm install
npm run build
npm run link
cd $WORKDIR
mkdir -p /tmp/test-results/interop/
export MOCHA_FILE="$(mktemp /tmp/test-results/interop/unit.XXXXXX.xml)"
npx ipfs-interop -- -t node -f $(sed -n -e "s|^require('\(.*\)')$|test/\1|p" node_modules/ipfs-interop/test/node.js | circleci tests split) -- --reporter mocha-circleci-reporter
Expand All @@ -242,6 +251,9 @@ jobs:
LIBP2P_TCP_REUSEPORT: false
LIBP2P_ALLOW_WEAK_RSA_KEYS: 1
IPFS_GO_EXEC: /tmp/circleci-workspace/bin/ipfs
IPFS_JS_EXEC: /tmp/js-ipfs/packages/ipfs/src/cli.js
IPFS_JS_MODULE: /tmp/js-ipfs/packages/ipfs/dist/cjs/src/index.js
IPFS_JS_HTTP_MODULE: /tmp/js-ipfs/packages/ipfs-http-client/dist/cjs/src/index.js
- store_test_results:
path: /tmp/test-results
go-ipfs-api:
Expand Down
Loading

0 comments on commit a43e506

Please sign in to comment.