[http-api] Unable to decode pubsub message property from

[dignifiedquire]

Version information:

0.4.5-pre1

Type:

Bug

Description:

I am trying to decode the pubsub messages on the http-api in js-ipfs-api, but I am failing to get anything sensible out of the from property.

The value I'm getting as a string looks like this: '\u0012 �x\u0017D����;�\'sW\u0010�E�\u0007m�\u001bU��6bj�1�}�' but trying to decode it into a buffer using utf8 does not give anything sensible. Ideally the from parameter isn't encoded at all on the http-api level, and it would simply be the base58 encoded string of the multihash.

[mateon1]

From what you mentioned in IRC, the long B58 string you get with mh.toB58String(new Buffer(obj.from)) encodes the � characters improperly.
Decoding 4DLB4jBmN6ip4CUt1zHC5k16Aqf3QE5CPVu1uYRKpJV2c4qrjzy7Vvd5cgd6N9JJ4YSdpnGpC76oddAjsK from b58 to hex gives:
1220EFBFBDEFBFBD2E3729EFBFBDEFBFBD0CEFBFBDEFBFBD1D51EFBFBD1A124BEFBFBD0B240BEFBFBDEFBFBD77EFBFBD622FEFBFBD7A11EFBFBD3B76
Notice the repeating EFBFBD, that is the encoding for � characters, but from the multihash you have on IRC.

For comparison
Good multihash:
Qmb7Ncabz2JZ3eTELRyXThM8mB8cBqaLEF3nbWpwSFrVBT
1220BDC12E3729B0F70C8C881D51891A124B930B240BE9B677CF622F817A11D43B76
Bad multihash:
4DLB4jBmN6ip4CUt1zHC5k16Aqf3QE5CPVu1uYRKpJV2c4qrjzy7Vvd5cgd6N9JJ4YSdpnGpC76oddAjsK
1220EFBFBDEFBFBD2E3729EFBFBDEFBFBD0CEFBFBDEFBFBD1D51EFBFBD1A124BEFBFBD0B240BEFBFBDEFBFBD77EFBFBD622FEFBFBD7A11EFBFBD3B76

1220
BD -> EFBFBD
C1 -> EFBFBD
2E3729
B0 -> EFBFBD
F7 -> EFBFBD
0C
... And so on

It seems that all bytes >7F are converted to �

[dignifiedquire]

Did some more investigation, this is what curl gives me

curl -X GET -H "Content-type: application/json" -H "Accept: application/json" http://localhost:5001/api/v0/pubsub/sub\?arg\=hello
{}
{"from":"\u0012 \ufffd\ufffd\u0016\ufffd\ufffdR8\ufffd~\ufffd2\ufffd\ufffd(8N\ufffd\ufffd\ufffd\ufffd\ufffd|\ufffd\ufffd\ufffd\ufffd3\ufffd\ufffd\ufffd\ufffd4","data":"d29ybGQ=","seqno":"FJGw2+zYqZY=","topicIDs":["hello"]}

[dignifiedquire]

Running that string through decodeURI gives the mangled data we see above:

> decodeURI('\u0012 \ufffd\ufffd\u0016\ufffd\ufffdR8\ufffd~\ufffd2\ufffd\ufffd(8N\ufffd\ufffd\ufffd\ufffd\ufffd|\ufffd\ufffd\ufffd\ufffd3\ufffd\ufffd\ufffd\ufffd4')
'\u0012 ��\u0016��R8�~�2��(8N�����|����3����4'

[Kubuxu]

The corruption is due to the Protobuf defining the Message format of pub/floodsub using string type for the from filed. The messages contain in it binary representation of senders identity (binary multihash).

The generated Go files define this filed as string. Go's JSON marshaller converts only []byte type to base64 when encoding. This leads to corruption as the marshaller will try to interpret the string as UTF-8.

The Portobuf Guide says that string type should be only used for values that are valid UTF-8 strings or ASCI 7bit strings. This is not the case here.

There are few ways to proceed, the best one seems to be define this filed as bytes and then there are two options, we can include (at a cost of one byte) multibase byte there and thus be able to send the sender to be encoded in whichever base, but it will be always encoded again into base64 when sent over JSON.

Spec update for this: libp2p/pubsub-notes#6

[whyrusleeping]

Made the change in go-floodsub, should be fixed in the next update

[whyrusleeping]

@dignifiedquire can you check latest master and verify this has been resolved?

[dignifiedquire]

Will do later today

[daviddias]

PubSub tests are passing with js-ipfs-api and latest go-ipfs master

   callback API
      single node
        .publish
          ✓ errors on string messags
          ✓ message from buffer
        .subscribe
          ✓ to one topic
          ✓ attaches multiple event listeners
          ✓ discover options
      multiple nodes connected
        .peers
          ✓ does not error when not subscribed to a topic
          - doesn't return extra peers
          - returns peers for a topic - one peer
          ✓ lists peers for a topic - multiple peers (507ms)
        .ls
          ✓ empty list when no topics are subscribed
          ✓ list with 1 subscribed topic
          ✓ list with 3 subscribed topics
        multiple nodes
          ✓ receive messages from different node (505ms)
          ✓ receive multiple messages (506ms)
        load tests
          ✓ call publish 1k times (462ms)
Send/Receive 10k messages took: 5042 ms, 1983 ops / s

          ✓ send/receive 10k messages (5548ms)
          ✓ call subscribe/unsubscribe 1k times
    promise API

      ✓ .subscribe and .publish
      ✓ .peers
      ✓ .ls

However, now I'm getting the name.resolve tests to fail due to a duplicated /ipfs/ prefix, see:

  1) .name .name.resolve:

      Uncaught AssertionError: expected { Object (Path) } to deeply equal { Object (Path) }
      + expected - actual

       {
      -  "Path": "/ipfs/Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP"
      +  "Path": "/ipfs//ipfs/Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP"
       }

      at apiClients.a.name.resolve (test/interface-ipfs-core/name.spec.js:45:25)
      at f (node_modules/once/once.js:25:25)
      at err (src/stream-to-json-value.js:30:5)
      at done (src/stream-to-value.js:10:26)
      at ConcatStream.<anonymous> (node_modules/concat-stream/index.js:36:43)
      at finishMaybe (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:513:14)
      at afterWrite (node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:395:3)
      at _combinedTickCallback (internal/process/next_tick.js:80:20)
      at process._tickDomainCallback (internal/process/next_tick.js:122:9)

  2) .name promise .name.resolve:

      AssertionError: expected { Object (Path) } to deeply equal { Object (Path) }
      + expected - actual

       {
      -  "Path": "/ipfs/Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP"
      +  "Path": "/ipfs//ipfs/Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP"
       }

      at apiClients.a.name.resolve.then (test/interface-ipfs-core/name.spec.js:65:29)
      at process._tickDomainCallback (internal/process/next_tick.js:129:7)

[whyrusleeping]

@diasdavid can this issue be closed then?

[dignifiedquire]

Not yet, the tests were disabled: https://github.com/ipfs/interface-ipfs-core/blob/master/src/pubsub.js#L116-L117 so you need to first reenable them