ipld / go-ipld-prime

Golang interfaces for the IPLD Data Model, with core Codecs included, IPLD Schemas support, and some handy functional transforms tools.
MIT License
132 stars 50 forks source link

bindnode doesn't calculate map `Length()` properly when properties are absent #332

Closed rvagg closed 2 years ago

rvagg commented 2 years ago

Can be seen @ https://github.com/ipfs/go-graphsync/pull/332

Relevant bit of schema looks like:

type GraphSyncRequest struct {
  id                GraphSyncRequestID  (rename "ID")   # unique id set on the requester side
  root     optional Link                (rename "Root") # a CID for the root node in the query
  selector optional Any                 (rename "Sel")  # see https://github.com/ipld/specs/blob/master/selectors/selectors.md
  extensions        GraphSyncExtensions (rename "Ext")  # side channel information
  priority          GraphSyncPriority   (rename "Pri")  # the priority (normalized). default to 1
  cancel            Bool                (rename "Canc") # whether this cancels a request
  update            Bool                (rename "Updt") # whether this is an update to an in progress request
} representation map

So, using pointers for the 2 optional fields. But then when encoding one of these, with nil values, to dag-cbor I get:

a364426c6b7380645265717381a76249445020109ec634284a19a31e56de508bd0f063457874a063507269006443616e63f56455706474f4645273707380

Which, looks like this:

A3                                      # map(3)
   64                                   # text(4)
      426C6B73                          # "Blks"
   80                                   # array(0)
   64                                   # text(4)
      52657173                          # "Reqs"
   81                                   # array(1)
      A7                                # map(7)
         62                             # text(2)
            4944                        # "ID"
         50                             # bytes(16)
            2E039BD0020B4AE59E8983A201E5A777 # ".\x03\x9B\xD0\x02\vJ\xE5\x9E\x89\x83\xA2\x01\xE5\xA7w"
         63                             # text(3)
            457874                      # "Ext"
         A0                             # map(0)
         63                             # text(3)
            507269                      # "Pri"
         00                             # unsigned(0)
         64                             # text(4)
            43616E63                    # "Canc"
         F5                             # primitive(21)
         64                             # text(4)
            55706474                    # "Updt"
         F4                             # primitive(20)
         64                             # text(4)
            52737073                    # "Rsps"
         80                             # array(0)

Which is invalid, thanks to map(7) - i.e., here's a map with 7 fields, but I'm only going to give you 5 of them. Hence the empty Rsps array pulled up into the request map as if it's a field, but it's still missing one more field even then.

Basically, the dag-cbor codec relies on Length() when writing a map(x) token, and bidnode is just reporting how many fields in its associated struct, discounting any that may be optional and missing, and then the map iterator is only returning 5 of them.

mvdan commented 2 years ago

This was fixed by https://github.com/ipld/go-ipld-prime/commit/1930bab51e48a8dcee8b76b9a2b4cc3868ac8d29 - I forgot to link the issue from the commit message.