-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
/api/v0/pubsub/pub should use body for payload #8454
Labels
topic/docs
Documentation
Comments
Indeed. Things changed since 2020 but js-ipfs-http-client seems to send data in body instead of second Need to double check if/how this impacts #8183 |
lidel
added
need/triage
Needs initial labeling and prioritization
topic/docs
Documentation
labels
Sep 21, 2021
lidel
added a commit
to coryschwartz/go-ipfs
that referenced
this issue
Oct 15, 2021
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 ipfs#8454 and makes rpc in go-ipfs easier to code against.
lidel
added a commit
to coryschwartz/go-ipfs
that referenced
this issue
Oct 25, 2021
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 ipfs#8454 and makes rpc in go-ipfs easier to code against.
lidel
added a commit
that referenced
this issue
Nov 29, 2021
* 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 <lidel@lidel.org> Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
https://docs.ipfs.io/reference/api/http/#api-v0-pubsub-pub states that payload should go in a querystring param called
arg
while i think this somehow works with some serialisation it should say to use the request body instead, just like the dht/put https://docs.ipfs.io/reference/api/http/#api-v0-dht-putWe are updating js-ipfs-http-client to use body instead of querystring in this PR ipfs/js-ipfs#3013.
The text was updated successfully, but these errors were encountered: