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.

Sign up for GitHub

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

Jump to bottom

IndexSorted should not load CIDs with multihash code of IDENTITY #215

Closed
masih opened this issue Aug 30, 2021 · 2 comments · Fixed by #218
Closed

IndexSorted should not load CIDs with multihash code of IDENTITY #215

masih opened this issue Aug 30, 2021 · 2 comments · Fixed by #218

Comments

@masih
Copy link
Member

masih commented Aug 30, 2021 •
edited
Loading

It is possible for a CAR file to contain CIDs with multihash IDENTITY. Such CIDs could significantly bloat the index size and cause OOM error. As stated in the CARv2 spec Index should disallow them and safely skip over them when indexing a CAR file.

IndexSorted currently loads all given CIDs. Skip over CIDs with multihash IDENTITY.
@masih masih mentioned this issue Sep 1, 2021
Merged
@rvagg
Copy link
Member

rvagg commented Sep 2, 2021

Indeedy, we even included this in the spec and made it bold:

Indexes should not include identity hash CIDs. It is assumed that any use of a CARv2 as a blockstore will return identity CID data immediately by extracting it from the CID, therefore there should be no need to provide indexing for such entries.

@masih
Copy link
Member Author

masih commented Sep 2, 2021

The new index type does this already; I'll get this sorted in the IndexSorted type shortly

@masih masih changed the title CIDs with multihash code of IDENTITY IndexSorted should not load CIDs with multihash code of IDENTITY Sep 2, 2021
masih added a commit that referenced this issue Sep 2, 2021
@masih
Ignore records with IDENTITY CID in IndexSorted
a9e86b9 
Skip `IDENTITY` CIDs in `IndexSorted`, as required by CARv2
specification.

Re-generate `sample-wrapped-v2.car` testdata, since its previous index
contained CIDs with `IDENTITY` multihash.

Gracefully handle CIDs with `IDENTITY` multihash code in the
`blockstore` API. Since `Get`, `GetSize` and `Has` interfaces rely on
the index, decode given keys directly to satisfy calls for such CIDs.

See:
- https://ipld.io/specs/transport/car/carv2/#index-format

Fixes #215
@masih masih mentioned this issue Sep 2, 2021
Merged
masih added a commit that referenced this issue Sep 7, 2021
@masih
Ignore records with IDENTITY CID in IndexSorted
c09aef7 
Skip `IDENTITY` CIDs in `IndexSorted`, as required by CARv2
specification.

Re-generate `sample-wrapped-v2.car` testdata, since its previous index
contained CIDs with `IDENTITY` multihash.

Gracefully handle CIDs with `IDENTITY` multihash code in the
`blockstore` API. Since `Get`, `GetSize` and `Has` interfaces rely on
the index, decode given keys directly to satisfy calls for such CIDs.

See:
- https://ipld.io/specs/transport/car/carv2/#index-format

Fixes #215
masih added a commit that referenced this issue Sep 7, 2021
@masih
Ignore records with IDENTITY CID in IndexSorted
3dfd1c2 
Skip `IDENTITY` CIDs in `IndexSorted`, as required by CARv2
specification.

Re-generate `sample-wrapped-v2.car` testdata, since its previous index
contained CIDs with `IDENTITY` multihash.

Gracefully handle CIDs with `IDENTITY` multihash code in the
`blockstore` API. Since `Get`, `GetSize` and `Has` interfaces rely on
the index, decode given keys directly to satisfy calls for such CIDs.

See:
- https://ipld.io/specs/transport/car/carv2/#index-format

Fixes #215
masih added a commit that referenced this issue Sep 8, 2021
@masih
Ignore records with IDENTITY CID in IndexSorted
8b7eb8b 
Skip `IDENTITY` CIDs in `IndexSorted`, as required by CARv2
specification.

Re-generate `sample-wrapped-v2.car` testdata, since its previous index
contained CIDs with `IDENTITY` multihash.

Gracefully handle CIDs with `IDENTITY` multihash code in the
`blockstore` API. Since `Get`, `GetSize` and `Has` interfaces rely on
the index, decode given keys directly to satisfy calls for such CIDs.

See:
- https://ipld.io/specs/transport/car/carv2/#index-format

Fixes #215
masih added a commit that referenced this issue Sep 8, 2021
@masih
Ignore records with IDENTITY CID in IndexSorted
c9d8b61 
Skip `IDENTITY` CIDs in `IndexSorted`, as required by CARv2
specification.

Re-generate `sample-wrapped-v2.car` testdata, since its previous index
contained CIDs with `IDENTITY` multihash.

Gracefully handle CIDs with `IDENTITY` multihash code in the
`blockstore` API. Since `Get`, `GetSize` and `Has` interfaces rely on
the index, decode given keys directly to satisfy calls for such CIDs.

See:
- https://ipld.io/specs/transport/car/carv2/#index-format

Fixes #215
masih added a commit that referenced this issue Sep 8, 2021
@masih
Ignore records with IDENTITY CID in IndexSorted
355d5e6 
Skip `IDENTITY` CIDs in `IndexSorted`, as required by CARv2
specification.

Re-generate `sample-wrapped-v2.car` testdata, since its previous index
contained CIDs with `IDENTITY` multihash.

Gracefully handle CIDs with `IDENTITY` multihash code in the
`blockstore` API. Since `Get`, `GetSize` and `Has` interfaces rely on
the index, decode given keys directly to satisfy calls for such CIDs.

See:
- https://ipld.io/specs/transport/car/carv2/#index-format

Fixes #215
@masih masih closed this as completed in #218 Sep 8, 2021
masih added a commit that referenced this issue Sep 8, 2021
@masih
Ignore records with IDENTITY CID in IndexSorted
acd3ead 
Skip `IDENTITY` CIDs in `IndexSorted`, as required by CARv2
specification.

Re-generate `sample-wrapped-v2.car` testdata, since its previous index
contained CIDs with `IDENTITY` multihash.

Gracefully handle CIDs with `IDENTITY` multihash code in the
`blockstore` API. Since `Get`, `GetSize` and `Has` interfaces rely on
the index, decode given keys directly to satisfy calls for such CIDs.

See:
- https://ipld.io/specs/transport/car/carv2/#index-format

Fixes #215
masih added a commit that referenced this issue Sep 17, 2021
@masih
Implement options to handle IDENTITY CIDs gracefully
d444560 
Implement two additional options that allow a CARv2 file to 1) include
IDENTNTIY CIDs, and 2) specify a maximum allowed CID length with default
of 2KiB as a sufficiently large default.

Configure ReadWrite blockstore to persist given blocks with IDENTITY
CIDs.

Introduce a new Characteristics filed that signalls whether an index in
a CAR file contains a full catalog of CIDs for backward compatibility
purposes. Note, this is a new addition and will need to be added to the
spec in a separate PR.

Relates to #215
@masih masih mentioned this issue Sep 17, 2021
Merged
masih added a commit that referenced this issue Sep 17, 2021
@masih
Implement options to handle IDENTITY CIDs gracefully
f1622e4 
Implement two additional options that allow a CARv2 file to 1) include
IDENTNTIY CIDs, and 2) specify a maximum allowed CID length with default
of 2KiB as a sufficiently large default.

Configure ReadWrite blockstore to persist given blocks with IDENTITY
CIDs.

Introduce a new Characteristics filed that signalls whether an index in
a CAR file contains a full catalog of CIDs for backward compatibility
purposes. Note, this is a new addition and will need to be added to the
spec in a separate PR.

Relates to #215
masih added a commit that referenced this issue Sep 17, 2021
@masih
Implement options to handle IDENTITY CIDs gracefully
9f9f687 
Implement two additional options that allow a CARv2 file to 1) include
IDENTNTIY CIDs, and 2) specify a maximum allowed CID length with default
of 2KiB as a sufficiently large default.

Configure ReadWrite blockstore to persist given blocks with IDENTITY
CIDs.

Introduce a new Characteristics filed that signalls whether an index in
a CAR file contains a full catalog of CIDs for backward compatibility
purposes. Note, this is a new addition and will need to be added to the
spec in a separate PR.

Relates to #215
masih added a commit that referenced this issue Sep 20, 2021
@masih
Implement options to handle IDENTITY CIDs gracefully
bdfd5f3 
Implement two additional options that allow a CARv2 file to 1) include
IDENTNTIY CIDs, and 2) specify a maximum allowed CID length with default
of 2KiB as a sufficiently large default.

Configure ReadWrite blockstore to persist given blocks with IDENTITY
CIDs.

Introduce a new Characteristics filed that signalls whether an index in
a CAR file contains a full catalog of CIDs for backward compatibility
purposes. Note, this is a new addition and will need to be added to the
spec in a separate PR.

Relates to #215
masih added a commit that referenced this issue Sep 20, 2021
@masih
Implement options to handle IDENTITY CIDs gracefully
92e3c89 
Implement two additional options that allow a CARv2 file to 1) include
IDENTNTIY CIDs, and 2) specify a maximum allowed CID length with default
of 2KiB as a sufficiently large default.

Configure ReadWrite blockstore to persist given blocks with IDENTITY
CIDs.

Introduce a new Characteristics filed that signalls whether an index in
a CAR file contains a full catalog of CIDs for backward compatibility
purposes. Note, this is a new addition and will need to be added to the
spec in a separate PR.

Relates to #215
masih added a commit that referenced this issue Sep 20, 2021
@masih
Implement options to handle IDENTITY CIDs gracefully
023c842 
Implement two additional options that allow a CARv2 file to 1) include
IDENTNTIY CIDs, and 2) specify a maximum allowed CID length with default
of 2KiB as a sufficiently large default.

Configure ReadWrite blockstore to persist given blocks with IDENTITY
CIDs.

Introduce a new Characteristics filed that signalls whether an index in
a CAR file contains a full catalog of CIDs for backward compatibility
purposes. Note, this is a new addition and will need to be added to the
spec in a separate PR.

Relates to #215
masih added a commit that referenced this issue Sep 20, 2021
@masih
Implement options to handle IDENTITY CIDs gracefully
29405e4 
Implement two additional options that allow a CARv2 file to 1) include
IDENTNTIY CIDs, and 2) specify a maximum allowed CID length with default
of 2KiB as a sufficiently large default.

Configure ReadWrite blockstore to persist given blocks with IDENTITY
CIDs.

Introduce a new Characteristics filed that signalls whether an index in
a CAR file contains a full catalog of CIDs for backward compatibility
purposes. Note, this is a new addition and will need to be added to the
spec in a separate PR.

Relates to #215
masih added a commit that referenced this issue Sep 20, 2021
@masih
Implement options to handle IDENTITY CIDs gracefully
f35d88c 
Implement two additional options that allow a CARv2 file to 1) include
IDENTNTIY CIDs, and 2) specify a maximum allowed CID length with default
of 2KiB as a sufficiently large default.

Configure ReadWrite blockstore to persist given blocks with IDENTITY
CIDs.

Introduce a new Characteristics filed that signalls whether an index in
a CAR file contains a full catalog of CIDs for backward compatibility
purposes. Note, this is a new addition and will need to be added to the
spec in a separate PR.

Relates to #215
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ignore records with IDENTITY CID in IndexSorted ipld/go-car
2 participants
@masih @rvagg