Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
IPIP-402: Partial CAR Support on Trustless Gateways #402
IPIP-402: Partial CAR Support on Trustless Gateways #402
Changes from 7 commits
dd84771
6f7f7b3
aeff21c
b5eca27
c68f06a
1fc8915
98191b5
a6bfb2c
78e81e0
bfb5d3c
0639715
56786ff
61d1088
e1af6e4
f5f33ff
4ebbb96
e3c36f3
0953cb6
ffb1550
584098d
917efb9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh look! Real-world use proving this problem: ipfs/js-ipfs-unixfs#335
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rvagg dag-pb TSize has nothing to do with
entity-bytes
at all. The UnixFSfilesize
andblocksizes
(protobuf elements 3 and 4 of the Data field of the dag-pb nodes for UnixFS files) does.AFAIK the only thing TSize is potentially used by for anyone in the context of the HTTP Gateway API is in the context of the trusted component of the API and giving rough estimates of file/directory sizes in directory HTMLs when the gateway doesn't want to use resources to grab the first block of each directory element to figure out if it's a file/directory and for files what the
filesize
is. However, what goes into a directory HTML file is non-normative (seespecs/src/http-gateways/path-gateway.md
Line 682 in add659b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, different value, same problem though; the value is encoded by the producer of the blocks and has to be implicitly trusted to get the right byte range
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, the issue with malformed graphs still exists along with the trust problems you mentioned. It shows up here, but it also likely belongs as an implementers note on the trusted gateway spec as well.
I was just calling out that:
filesize
andblocksizes
have actual definitions which means when they're not obeyed the UnixFS graph is malformed unlike the argument about TSize where people seem more hesitant to give it an actual definition (although it seems to have been defined as cumulative graph size since day 1 ipfs/go-merkledag@63e4477#diff-10837cc3557cec0045183193a03f17af589591f2be0753262027534bb8f64ad9R38-R39)Edit: Also, IIUC because
filesize
andblocksizes
have actual definitions implementations don' t actually have to fall into this trap (even if common ones today do). As long asReadAll(<UnixFS file cid>)[X:Y]
matchesRead(<UnixFS file cid>, X,Y)
this problem goes away. So this isn't really a spec issue here, or necessarily in UnixFS (although notes to implementers seem reasonable), since these issues only happen with malformed UnixFS data.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
filesize
/blocksizes
note in 0953cb6, but agree, the implicit trust in DAG being correctly created is a concern beyond this IPIP or even this API.It is the same class of problems as "I do trust data match the CID i requested, but where do I get trusted CIDs from?" – IMO beyond the scope of this IPIP.