Skip to content
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

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

Closed
rvagg opened this issue Jan 14, 2022 · 1 comment
Closed

Comments

@rvagg
Copy link
Member

rvagg commented Jan 14, 2022

Can be seen @ ipfs/go-graphsync#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
Copy link
Contributor

mvdan commented Jan 20, 2022

This was fixed by 1930bab - I forgot to link the issue from the commit message.

@mvdan mvdan closed this as completed Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants