Validate size in the DagReaders

[Stebalien]

When reading files, we should always validate the size (it doesn't look like it currently does). If we ever hit a chunk that's too large, we should truncate it. If we ever hit a chunk that's too small, we should return zeros (allowing holes in files is actually quite useful). We should also document this in a spec somewhere.

Expected size not matching the actual size of something tends to lead to security problems...

@diasdavid objections? How does js-ipfs deal with this issue?

[daviddias]

@Stebalien you mean just for unixfs for now right? Other IPLD graphs won't have necessarily a Size property.

I've no objections. It won't be a strong guarantee though bad graphs can still show up.

[Stebalien]

@diasdavid yes, just unixfs. I picked that algorithm so we can handle bad sizes on the fly without having to check anything up-front.

[kevina]

I am not 100% sure what needs to be done, but I think I can handle this. I will have a better idea once I look at the code more closely.

[daviddias]

//cc @pgte and @vmx for the JS side

[kevina]

@Stebalien I am a little confused about the semantic, for example in #4447 what do you think is the expected (if not correct) behavior.

In particular the raw leaf zb2rhX9DvypVUCrp1SyoB6JpvdNEGcujJm7RZ6btt1zJBvnKq is 512 bytes but according to unixfs it should be 476 bytes (a hack), should this block be truncated to 476 bytes when read as part of QmbTswUf339CVJsqB6rSeNum7hrnUsNqFbgkth8g3u2chX?

[ivan386]

https://github.com/ipfs/go-ipfs/blob/3106135dd5d081e1c0a49e22a435463f32412eb7/unixfs/io/pbdagreader.go#L239

@kevina yes. If don't extend or cut data to block size (476 bytes in this case) use of Seek() give different data in same position in file.

[ivan386]

Block can have more or less links than blocksizes.

[Stebalien]

@kevina

Just to be on the same page, there are two "sizes":

1. The "Size" field on links.
2. The blocksizes/filesizes fields.

I'm more concerned with the second as it affects, e.g., seeking. I think we can treat the first one as optional.

So, if we have a raw leaf that's part of a file and the file states that the "blocksize" should be 476, I'd truncate/zero-extend the leaf to 476 bytes as necessary. We could also treat this as an error and declare that part of the file "missing". My motivation for truncating/zero-extending if the size doesn't match is that it allows us to truncate/zero extend without copying the block (i.e., the truncate syscall).

However, at the end of the day, I really just care about the following requirements:

1. We never have to download blocks we don't need just so we can validate something.
2. We're consistent across all access patterns.

I am a little confused about the semantic, for example in #4447 what do you think is the expected (if not correct) behavior.

I assume you meant to point to a different issue.

[kevina]

@Stebalien sorry meant to point to #4667

[ivan386]

@Stebalien @kevina
Here: ivan386@ef35df6

1. If link or blocksize don't have pair it ignored
2. If link has less data then blocksize it appended by zeros
3. If link has more data then blocksize it truncated
4. Data will be appended or truncated to filesize

I think about align blocks to end of file. Then we can do hole between header data and first link.

[kevina]

@ivan386 I am also working on a solution that uses a slightly different approach that I should have ready later today