diff --git a/.circleci/main.yml b/.circleci/main.yml index 061bffac86e..8139b97d7dd 100644 --- a/.circleci/main.yml +++ b/.circleci/main.yml @@ -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 @@ -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: diff --git a/core/commands/pubsub.go b/core/commands/pubsub.go index 1ec5016d6fd..47d893852af 100644 --- a/core/commands/pubsub.go +++ b/core/commands/pubsub.go @@ -2,13 +2,15 @@ package commands import ( "context" - "encoding/binary" "fmt" "io" + "io/ioutil" "net/http" "sort" cmdenv "github.com/ipfs/go-ipfs/core/commands/cmdenv" + mbase "github.com/multiformats/go-multibase" + "github.com/pkg/errors" cmds "github.com/ipfs/go-ipfs-cmds" options "github.com/ipfs/interface-go-ipfs-core/options" @@ -21,10 +23,11 @@ var PubsubCmd = &cmds.Command{ ipfs pubsub allows you to publish messages to a given topic, and also to subscribe to new messages on a given topic. -This is an experimental feature. It is not intended in its current state -to be used in a production environment. +EXPERIMENTAL FEATURE -To use, the daemon must be run with '--enable-pubsub-experiment'. + It is not intended in its current state to be used in a production + environment. To use, the daemon must be run with + '--enable-pubsub-experiment'. `, }, Subcommands: map[string]*cmds.Command{ @@ -35,14 +38,10 @@ To use, the daemon must be run with '--enable-pubsub-experiment'. }, } -const ( - pubsubDiscoverOptionName = "discover" -) - type pubsubMessage struct { - From []byte `json:"from,omitempty"` - Data []byte `json:"data,omitempty"` - Seqno []byte `json:"seqno,omitempty"` + From string `json:"from,omitempty"` + Data string `json:"data,omitempty"` + Seqno string `json:"seqno,omitempty"` TopicIDs []string `json:"topicIDs,omitempty"` } @@ -52,37 +51,42 @@ var PubsubSubCmd = &cmds.Command{ ShortDescription: ` ipfs pubsub sub subscribes to messages on a given topic. -This is an experimental feature. It is not intended in its current state -to be used in a production environment. +EXPERIMENTAL FEATURE -To use, the daemon must be run with '--enable-pubsub-experiment'. -`, - LongDescription: ` -ipfs pubsub sub subscribes to messages on a given topic. + It is not intended in its current state to be used in a production + environment. To use, the daemon must be run with + '--enable-pubsub-experiment'. -This is an experimental feature. It is not intended in its current state -to be used in a production environment. +PEER ENCODING -To use, the daemon must be run with '--enable-pubsub-experiment'. + Peer IDs in From fields are encoded using the default text representation + from go-libp2p. This ensures the same string values as in 'ipfs pubsub peers'. -This command outputs data in the following encodings: - * "json" -(Specified by the "--encoding" or "--enc" flag) +TOPIC AND DATA ENCODING + + Topics, Data and Seqno are binary data. To ensure all bytes are transferred + correctly the RPC client and server will use multibase encoding behind + the scenes. + + You can inspect the format by passing --enc=json. The ipfs multibase commands + can be used for encoding/decoding multibase strings in the userland. `, }, Arguments: []cmds.Argument{ - cmds.StringArg("topic", true, false, "String name of topic to subscribe to."), - }, - Options: []cmds.Option{ - cmds.BoolOption(pubsubDiscoverOptionName, "Deprecated option to instruct pubsub to discovery peers for the topic. Discovery is now built into pubsub."), + cmds.StringArg("topic", true, false, "Name of topic to subscribe to."), }, + PreRun: urlArgsEncoder, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { api, err := cmdenv.GetApi(env, req) if err != nil { return err } + if err := urlArgsDecoder(req, env); err != nil { + return err + } topic := req.Arguments[0] + sub, err := api.PubSub().Subscribe(req.Context, topic) if err != nil { return err @@ -101,33 +105,39 @@ This command outputs data in the following encodings: return err } - if err := res.Emit(&pubsubMessage{ - Data: msg.Data(), - From: []byte(msg.From()), - Seqno: msg.Seq(), - TopicIDs: msg.Topics(), - }); err != nil { + // turn bytes into strings + encoder, _ := mbase.EncoderByName("base64url") + psm := pubsubMessage{ + Data: encoder.Encode(msg.Data()), + From: msg.From().Pretty(), + Seqno: encoder.Encode(msg.Seq()), + } + for _, topic := range msg.Topics() { + psm.TopicIDs = append(psm.TopicIDs, encoder.Encode([]byte(topic))) + } + if err := res.Emit(&psm); err != nil { return err } } }, Encoders: cmds.EncoderMap{ cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, psm *pubsubMessage) error { - _, err := w.Write(psm.Data) + _, dec, err := mbase.Decode(psm.Data) + if err != nil { + return err + } + _, err = w.Write(dec) return err }), + // DEPRECATED, undocumented format we used in tests, but not anymore + // \n\n "ndpayload": cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, psm *pubsubMessage) error { - psm.Data = append(psm.Data, '\n') - _, err := w.Write(psm.Data) - return err + return errors.New("--enc=ndpayload was removed, use --enc=json instead") }), + // DEPRECATED, uncodumented format we used in tests, but not anymore + // "lenpayload": cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, psm *pubsubMessage) error { - buf := make([]byte, 8, len(psm.Data)+8) - - n := binary.PutUvarint(buf, uint64(len(psm.Data))) - buf = append(buf[:n], psm.Data...) - _, err := w.Write(buf) - return err + return errors.New("--enc=lenpayload was removed, use --enc=json instead") }), }, Type: pubsubMessage{}, @@ -135,40 +145,56 @@ This command outputs data in the following encodings: var PubsubPubCmd = &cmds.Command{ Helptext: cmds.HelpText{ - Tagline: "Publish a message to a given pubsub topic.", + Tagline: "Publish data to a given pubsub topic.", ShortDescription: ` ipfs pubsub pub publishes a message to a specified topic. +It reads binary data from stdin or a file. + +EXPERIMENTAL FEATURE + + It is not intended in its current state to be used in a production + environment. To use, the daemon must be run with + '--enable-pubsub-experiment'. -This is an experimental feature. It is not intended in its current state -to be used in a production environment. +HTTP RPC ENCODING + + The data to be published is sent in HTTP request body as multipart/form-data. + + Topic names are binary data too. To ensure all bytes are transferred + correctly via URL params, the RPC client and server will use multibase + encoding behind the scenes. -To use, the daemon must be run with '--enable-pubsub-experiment'. `, }, Arguments: []cmds.Argument{ cmds.StringArg("topic", true, false, "Topic to publish to."), - cmds.StringArg("data", true, true, "Payload of message to publish.").EnableStdin(), + cmds.FileArg("data", true, false, "The data to be published.").EnableStdin(), }, + PreRun: urlArgsEncoder, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { api, err := cmdenv.GetApi(env, req) if err != nil { return err } + if err := urlArgsDecoder(req, env); err != nil { + return err + } topic := req.Arguments[0] - err = req.ParseBodyArgs() + // read data passed as a file + file, err := cmdenv.GetFileArg(req.Files.Entries()) if err != nil { return err } - - for _, data := range req.Arguments[1:] { - if err := api.PubSub().Publish(req.Context, topic, []byte(data)); err != nil { - return err - } + defer file.Close() + data, err := ioutil.ReadAll(file) + if err != nil { + return err } - return nil + // publish + return api.PubSub().Publish(req.Context, topic, data) }, } @@ -178,10 +204,20 @@ var PubsubLsCmd = &cmds.Command{ ShortDescription: ` ipfs pubsub ls lists out the names of topics you are currently subscribed to. -This is an experimental feature. It is not intended in its current state -to be used in a production environment. +EXPERIMENTAL FEATURE + + It is not intended in its current state to be used in a production + environment. To use, the daemon must be run with + '--enable-pubsub-experiment'. -To use, the daemon must be run with '--enable-pubsub-experiment'. +TOPIC ENCODING + + Topic names are a binary data. To ensure all bytes are transferred + correctly RPC client and server will use multibase encoding behind + the scenes. + + You can inspect the format by passing --enc=json. ipfs multibase commands + can be used for encoding/decoding multibase strings in the userland. `, }, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { @@ -195,15 +231,35 @@ To use, the daemon must be run with '--enable-pubsub-experiment'. return err } + // emit topics encoded in multibase + encoder, _ := mbase.EncoderByName("base64url") + for n, topic := range l { + l[n] = encoder.Encode([]byte(topic)) + } + return cmds.EmitOnce(res, stringList{l}) }, Type: stringList{}, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(multibaseDecodedStringListEncoder), }, } -func stringListEncoder(req *cmds.Request, w io.Writer, list *stringList) error { +func multibaseDecodedStringListEncoder(req *cmds.Request, w io.Writer, list *stringList) error { + for n, mb := range list.Strings { + _, data, err := mbase.Decode(mb) + if err != nil { + return err + } + list.Strings[n] = string(data) + } + return safeTextListEncoder(req, w, list) +} + +// converts list of strings to text representation where each string is placed +// in separate line with non-printable/unsafe characters escaped +// (this protects terminal output from being mangled by non-ascii topic names) +func safeTextListEncoder(req *cmds.Request, w io.Writer, list *stringList) error { for _, str := range list.Strings { _, err := fmt.Fprintf(w, "%s\n", cmdenv.EscNonPrint(str)) if err != nil { @@ -218,23 +274,37 @@ var PubsubPeersCmd = &cmds.Command{ Tagline: "List peers we are currently pubsubbing with.", ShortDescription: ` ipfs pubsub peers with no arguments lists out the pubsub peers you are -currently connected to. If given a topic, it will list connected -peers who are subscribed to the named topic. +currently connected to. If given a topic, it will list connected peers who are +subscribed to the named topic. + +EXPERIMENTAL FEATURE -This is an experimental feature. It is not intended in its current state -to be used in a production environment. + It is not intended in its current state to be used in a production + environment. To use, the daemon must be run with + '--enable-pubsub-experiment'. -To use, the daemon must be run with '--enable-pubsub-experiment'. +TOPIC AND DATA ENCODING + + Topic names are a binary data. To ensure all bytes are transferred + correctly RPC client and server will use multibase encoding behind + the scenes. + + You can inspect the format by passing --enc=json. ipfs multibase commands + can be used for encoding/decoding multibase strings in the userland. `, }, Arguments: []cmds.Argument{ - cmds.StringArg("topic", false, false, "topic to list connected peers of"), + cmds.StringArg("topic", false, false, "Topic to list connected peers of."), }, + PreRun: urlArgsEncoder, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { api, err := cmdenv.GetApi(env, req) if err != nil { return err } + if err := urlArgsDecoder(req, env); err != nil { + return err + } var topic string if len(req.Arguments) == 1 { @@ -256,6 +326,40 @@ To use, the daemon must be run with '--enable-pubsub-experiment'. }, Type: stringList{}, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(safeTextListEncoder), }, } + +// TODO: move to cmdenv? +// Encode binary data to be passed as multibase string in URL arguments. +// (avoiding issues described in https://github.com/ipfs/go-ipfs/issues/7939) +func urlArgsEncoder(req *cmds.Request, env cmds.Environment) error { + encoder, _ := mbase.EncoderByName("base64url") + for n, arg := range req.Arguments { + req.Arguments[n] = encoder.Encode([]byte(arg)) + } + return nil +} + +// Decode binary data passed as multibase string in URL arguments. +// (avoiding issues described in https://github.com/ipfs/go-ipfs/issues/7939) +func urlArgsDecoder(req *cmds.Request, env cmds.Environment) error { + for n, arg := range req.Arguments { + encoding, data, err := mbase.Decode(arg) + if err != nil { + return errors.Wrap(err, "URL arg must be multibase encoded") + } + + // Enforce URL-safe encoding is used for data passed via URL arguments + // - without this we get data corruption similar to https://github.com/ipfs/go-ipfs/issues/7939 + // - we can't just deny base64, because there may be other bases that + // are not URL-safe – better to force base64url which is known to be + // safe in URL context + if encoding != mbase.Base64url { + return errors.New("URL arg must be base64url encoded") + } + + req.Arguments[n] = string(data) + } + return nil +} diff --git a/core/commands/swarm.go b/core/commands/swarm.go index 7cde3f38d43..7c7ee3e814f 100644 --- a/core/commands/swarm.go +++ b/core/commands/swarm.go @@ -453,7 +453,7 @@ var swarmAddrsLocalCmd = &cmds.Command{ }, Type: stringList{}, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(safeTextListEncoder), }, } @@ -485,7 +485,7 @@ var swarmAddrsListenCmd = &cmds.Command{ }, Type: stringList{}, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(safeTextListEncoder), }, } @@ -535,7 +535,7 @@ ipfs swarm connect /ip4/104.131.131.82/tcp/4001/p2p/QmaCpDMGvV2BGHeYERUEnRQAwe3N return cmds.EmitOnce(res, &stringList{output}) }, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(safeTextListEncoder), }, Type: stringList{}, } @@ -600,7 +600,7 @@ it will reconnect. return cmds.EmitOnce(res, &stringList{output}) }, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(safeTextListEncoder), }, Type: stringList{}, } @@ -722,7 +722,7 @@ Filters default to those specified under the "Swarm.AddrFilters" config key. return cmds.EmitOnce(res, &stringList{output}) }, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(safeTextListEncoder), }, Type: stringList{}, } @@ -778,7 +778,7 @@ var swarmFiltersAddCmd = &cmds.Command{ return cmds.EmitOnce(res, &stringList{added}) }, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(safeTextListEncoder), }, Type: stringList{}, } @@ -844,7 +844,7 @@ var swarmFiltersRmCmd = &cmds.Command{ return cmds.EmitOnce(res, &stringList{removed}) }, Encoders: cmds.EncoderMap{ - cmds.Text: cmds.MakeTypedEncoder(stringListEncoder), + cmds.Text: cmds.MakeTypedEncoder(safeTextListEncoder), }, Type: stringList{}, } diff --git a/test/sharness/t0180-pubsub.sh b/test/sharness/t0320-pubsub.sh similarity index 76% rename from test/sharness/t0180-pubsub.sh rename to test/sharness/t0320-pubsub.sh index 349e98d54c3..44ee8b58285 100755 --- a/test/sharness/t0180-pubsub.sh +++ b/test/sharness/t0320-pubsub.sh @@ -19,72 +19,72 @@ run_pubsub_tests() { PEERID_0=$(iptb attr get 0 id) && PEERID_2=$(iptb attr get 2 id) ' - + # ipfs pubsub sub test_expect_success 'pubsub' ' - echo "testOK" > expected && + echo -n -e "test\nOK" | ipfs multibase encode -b base64url > expected && touch empty && mkfifo wait || test_fsh echo init fail - + # ipfs pubsub sub is long-running so we need to start it in the background and # wait put its output somewhere where we can access it ( - ipfsi 0 pubsub sub --enc=ndpayload testTopic | if read line; then - echo $line > actual && + ipfsi 0 pubsub sub --enc=json testTopic | if read line; then + echo $line | jq -j .data > actual && echo > wait fi ) & ' - + test_expect_success "wait until ipfs pubsub sub is ready to do work" ' go-sleep 500ms ' - + test_expect_success "can see peer subscribed to testTopic" ' ipfsi 1 pubsub peers testTopic > peers_out ' - + test_expect_success "output looks good" ' echo $PEERID_0 > peers_exp && test_cmp peers_exp peers_out ' - - test_expect_success "publish something" ' - ipfsi 1 pubsub pub testTopic "testOK" &> pubErr + + test_expect_success "publish something from file" ' + echo -n -e "test\nOK" > payload-file && + ipfsi 1 pubsub pub testTopic payload-file &> pubErr ' - + test_expect_success "wait until echo > wait executed" ' cat wait && test_cmp pubErr empty && test_cmp expected actual ' - + test_expect_success "wait for another pubsub message" ' - echo "testOK2" > expected && + echo -n -e "test\nOK\r\n2" | ipfs multibase encode -b base64url > expected && mkfifo wait2 || test_fsh echo init fail - + # ipfs pubsub sub is long-running so we need to start it in the background and # wait put its output somewhere where we can access it ( - ipfsi 2 pubsub sub --enc=ndpayload testTopic | if read line; then - echo $line > actual && + ipfsi 2 pubsub sub --enc=json testTopic | if read line; then + echo $line | jq -j .data > actual && echo > wait2 fi ) & ' - + test_expect_success "wait until ipfs pubsub sub is ready to do work" ' go-sleep 500ms ' - - test_expect_success "publish something" ' - echo "testOK2" | ipfsi 3 pubsub pub testTopic &> pubErr + + test_expect_success "publish something from stdin" ' + echo -n -e "test\nOK\r\n2" | ipfsi 3 pubsub pub testTopic &> pubErr ' - + test_expect_success "wait until echo > wait executed" ' - echo "testOK2" > expected && cat wait2 && test_cmp pubErr empty && test_cmp expected actual @@ -93,7 +93,7 @@ run_pubsub_tests() { test_expect_success 'cleanup fifos' ' rm -f wait wait2 ' - + } # Normal tests @@ -114,7 +114,7 @@ startup_cluster $NUM_NODES --enable-pubsub-experiment test_expect_success 'set node 4 to listen on testTopic' ' rm -f node4_actual && - ipfsi 4 pubsub sub --enc=ndpayload testTopic > node4_actual & + ipfsi 4 pubsub sub --enc=json testTopic > node4_actual & ' run_pubsub_tests diff --git a/test/sharness/t0180-pubsub-gossipsub.sh b/test/sharness/t0321-pubsub-gossipsub.sh similarity index 76% rename from test/sharness/t0180-pubsub-gossipsub.sh rename to test/sharness/t0321-pubsub-gossipsub.sh index c5ff8daf39e..c89c8d1ae92 100755 --- a/test/sharness/t0180-pubsub-gossipsub.sh +++ b/test/sharness/t0321-pubsub-gossipsub.sh @@ -25,7 +25,7 @@ test_expect_success 'peer ids' ' ' test_expect_success 'pubsub' ' - echo "testOK" > expected && + echo -n -e "test\nOK" | ipfs multibase encode -b base64url > expected && touch empty && mkfifo wait || test_fsh echo init fail @@ -33,8 +33,8 @@ test_expect_success 'pubsub' ' # ipfs pubsub sub is long-running so we need to start it in the background and # wait put its output somewhere where we can access it ( - ipfsi 0 pubsub sub --enc=ndpayload testTopic | if read line; then - echo $line > actual && + ipfsi 0 pubsub sub --enc=json testTopic | if read line; then + echo $line | jq -j .data > actual && echo > wait fi ) & @@ -53,8 +53,9 @@ test_expect_success "output looks good" ' test_cmp peers_exp peers_out ' -test_expect_success "publish something" ' - ipfsi 1 pubsub pub testTopic "testOK" &> pubErr +test_expect_success "publish something from a file" ' + echo -n -e "test\nOK" > payload-file && + ipfsi 1 pubsub pub testTopic payload-file &> pubErr ' test_expect_success "wait until echo > wait executed" ' @@ -64,15 +65,15 @@ test_expect_success "wait until echo > wait executed" ' ' test_expect_success "wait for another pubsub message" ' - echo "testOK2" > expected && + echo -n -e "test\nOK2" | ipfs multibase encode -b base64url > expected && mkfifo wait2 || test_fsh echo init fail # ipfs pubsub sub is long-running so we need to start it in the background and # wait put its output somewhere where we can access it ( - ipfsi 2 pubsub sub --enc=ndpayload testTopic | if read line; then - echo $line > actual && + ipfsi 2 pubsub sub --enc=json testTopic | if read line; then + echo $line | jq -j .data > actual && echo > wait2 fi ) & @@ -83,11 +84,10 @@ test_expect_success "wait until ipfs pubsub sub is ready to do work" ' ' test_expect_success "publish something" ' - echo "testOK2" | ipfsi 1 pubsub pub testTopic &> pubErr + echo -n -e "test\nOK2" | ipfsi 1 pubsub pub testTopic &> pubErr ' test_expect_success "wait until echo > wait executed" ' - echo "testOK2" > expected && cat wait2 && test_cmp pubErr empty && test_cmp expected actual diff --git a/test/sharness/t0322-pubsub-http-rpc.sh b/test/sharness/t0322-pubsub-http-rpc.sh new file mode 100755 index 00000000000..4ecfefb6977 --- /dev/null +++ b/test/sharness/t0322-pubsub-http-rpc.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash + +test_description="Test pubsub command behavior over HTTP RPC API" + +. lib/test-lib.sh + +test_init_ipfs +test_launch_ipfs_daemon --enable-pubsub-experiment + +# Require topic as multibase +# https://github.com/ipfs/go-ipfs/pull/8183 +test_expect_success "/api/v0/pubsub/pub URL arg must be multibase encoded" ' + echo test > data.txt && + curl -s -X POST -F "data=@data.txt" "$API_ADDR/api/v0/pubsub/pub?arg=foobar" > result && + test_should_contain "error" result && + test_should_contain "URL arg must be multibase encoded" result +' + +# Use URL-safe multibase +# base64 should produce error when used in URL args, base64url should be used +test_expect_success "/api/v0/pubsub/pub URL arg must be in URL-safe multibase" ' + echo test > data.txt && + curl -s -X POST -F "data=@data.txt" "$API_ADDR/api/v0/pubsub/pub?arg=mZm9vYmFyCg" > result && + test_should_contain "error" result && + test_should_contain "URL arg must be base64url encoded" result +' + +test_kill_ipfs_daemon +test_done