fix: multibase in pubsub http rpc

[coryschwartz]

This PR got picked up by @lidel, who created below high level description:

This updates HTTP RPC wire format to wrap binary data in multibase to fix bug entire class of bugs described in #7939

TODO

• sharness
• go-ipfs-api
• go-ipfs-http-client
• interface-go-ipfs-core (update tests to be more robust - can be done async - we just need to ensure we test with topic/payload that has \n)
• decide if we prefer to keep public API unchanged, or switch topic names to be []byte
  • descoped. does not seem to be a useful change. wire format change has limited scope, and if someone uses client library in GO or JS they only need to update to latest version, no changes in code required. if we ALSO change the programmatic API in GO and JS that forces consumers to refactor their code, which is a separate thing that we can do in the future, but no need to do now.
• decide if we are ok with js-ipfs and js-ipfs-http-client returning peerid as CIDv1 while go-ipfs uses old notation
  • descoped. peerid is opaque string identifier, API design should make no assumption beyond this fact
• interop tests from fix: multibase in pubsub http rpc interop tests interop#387
  pubsub tests in ipfs/interop need JS and GO to act the same, so we need to land:
  • js-ipfs-http-client (use new wire format) – tracked in fix(pubsub): multibase in pubsub http rpc js-ipfs#3922
  • js-ipfs (use new wire format) – tracked in fix(pubsub): multibase in pubsub http rpc js-ipfs#3922
  • fix CI failing (https://github.com/ipfs/interop/runs/3965762343?check_suite_focus=true#step:6:3446)
  • then make sure interop tests pass:
    • GO-GO
    • JS-JS
    • JS-GO
    • GO-JS
    • disable pubsub interop tests on CI
      switching to npm version with fix: disable pubsub until http rpc changes land interop#388 so other PRs and master are not impacted.
    • enable pubsub interop tests:
      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

• Rather than newline-delimeted string args, data passed on stdin for ipfs pubsub pub gets wrapped in multibase
• data, topic, and seqno are expected to be multibase encoded on the HTTP API

multibase 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.

[aschmahmann]

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.

The data isn't actually base64 encoded, what's happening is that we emit a pubSubMessage https://github.com/ipfs/go-ipfs/pull/8183/files#diff-88c3196f5f326c7b97a3be71eab971648fabab00f6f8b9e78a238f43aa99ef50L42-L47 which has fields of types []byte and string where everything except for the Topic is []byte. Go's JSON marshaller automatically base64 encodes fields of type []byte while leaving strings alone.

The results are:

1. If topic names aren't UTF-8 encoded then you end up with strings interspersed with text like \u1234 which are no fun for javascript
2. The []byte fields are implicitly base64 encoded instead of being more explicitly multibase encoded

The results from #7939 (comment) are that we're going to be making a breaking change to the return types here. This means we can change pubSubMessage to return all strings and do the multibase encoding ourselves (base64 still good for space preservation).

Side note: @lidel @achingbrain do you have a preference for the returned multibase encoding? Base64URL, Base64, something else?

[lidel]

We need to require base64url for URL params, but for everything else (eg. response) base64 or base64url is fine (JS has built-in support for base64).

Might be easier to use base64url everywhere, so MVP client implementers have a single method for encoding/decoding.

[lidel]

Thank you @coryschwartz, it solves the problem described in #7939 (comment) and we no longer see bogus message splits on \n:

$ echo -ne 'he\nllo' | ipfs pubsub pub example-multibase 

produces single msg:

$ ipfs pubsub sub example-multibase --enc=json
{"from":"mACQIARIgR7j6xSOlG24jauZSt48b7emK7exFRxoaJvIFwidZFdc","data":"maGUKbGxv","seqno":"mFojhcYxl+kA","topicIDs":["mZXhhbXBsZS1tdWx0aWJhc2U"]

Remaining work here:

• switch url args to base64url (see comment inline)
• fix TestPubSub by updating things around go-ipfs-api/pubsub.go#L34
• fix ipfs/interop tests: pubsub.js & ipns-pubsub.js are failing
  • 👉 ❓ @achingbrain @aschmahmann this is chicken-vs-egg problem, not sure how to handle this.
    • Should we descope it from this PR and tackle as part of JS tribute work?
      We need to update js-ipfs and js-ipfs-http-client anyway, fixing interop tests could be part of that work.
    • Is it ok for interop tests to fail in the meantime, or should we park this PR until JS side (incl. interop tests) is done?

[lidel]

@aschmahmann GO side of things is ready for review.

TLDR changes:

• ipfs pubsub commands use multibase for topic passed in URL as arg and for data and seqno fields in topic messages
• data is sent in HTTP POST body as multipart, reusing existing file abstractions
• ipfs pubsub pub CLI changed and now reads data from file, instead of second CLI argument, and we no longer allow variadic payloads (only one message per API call).
• pubsub tests that test the real wire format (json)

Before we merge this PR we need to review and merge ipfs/go-ipfs-api#255 and ipfs/go-ipfs-http-client#151 and update this PR (see comments inline)

[lidel]

ℹ️ 💭
StringArg is the rot cause of data corruption and we arew unable to fix it, because it is a feature. Each line is new input (it interprets \n and \r\n as "message end" and splits payload when matching bytes arrive).

I decided to switch to FileArg instead of inventing a new Arg type for the same of keeping things easier to merge, review, and support (we already have File support in both JS and GO)

[lidel]

ℹ️ added \n all over the place (here, and other PRs) to ensure we don't have regressions and bugs like one described in #7939

[lidel]

ℹ️ this PR removes use of custom ndpayload format that was used ... only in tests 🙃 (and we had no tests for JSON.. now we do).