search

ipld / js-dag-json

JSON Directed Acyclic Graph for IPLD
Other
16 stars 6 forks source link

encoding object with `/` field should drop other feilds #86

Open Gozala opened 1 year ago

Gozala commented 1 year ago

Here is an example code to reproduce the problem:

const block = json.encode({
   hello: {
     '/': 'baguqeeratmwuhl736sndm4bi34xbifhyjqhatgwjrq6vjkfiafl7253rv4sq',
     message: 'hello'
   }
})

const data = json.decode(block) // Error: Invalid encoded CID form

I would expect encoder to just ignore message field of the hello and encode it as link as opposed to encoding it into an invalid block.

rvagg commented 1 year ago

Background re decoding, see https://ipld.io/specs/codecs/dag-json/spec/#parse-rejection-modes-in-the-reserved-namespace - one of the main reasons for the strictness around the "/" reserved space is the difficulty in tokenized parsing and needing to buffer tokens in order to make decisions about what you're dealing with.

Re encoding .. yeah that's an interesting case and you're right that we probably should be not allowing encoding of invalid forms!

I'm not in favour of silently dropping fields, however. I think in this case it should error on encode rather than apparently succeeding but losing some of your data. The "this is not right" signal should bubble up from the encoder to be dealt with by a different layer accordingly. If you want a layer to massage invalid data into an encodable form then that's fine but I don't think it belongs directly in the encode() of the codec - something like dag-pb's prepare() might be OK for that. Similar to the discussion in https://github.com/ipld/js-dag-cbor/issues/57.