Bugs related to codecs that return EOF on encode/decode (EOF error are ignored by CLI so we would need to either fix that or wrap the errors)

[warpfork]

[warpfork]

@aschmahmann suggested codecs may be returning EOF when they shouldn't? Or maybe they should be returning UnexpectedEOF rather than EOF in some cases?

I do believe there may be upstream bugs here, but info on which codecs, and what exactly the misbehavior is, would help.

[warpfork]

Possibly relatedly, I've also recently found at least one bug upstream where a codec returns not enough EOF errors: ipld/go-ipld-prime#230

[aschmahmann]

A couple of examples of this are:

- `echo "test" | ipfs dag put --input-enc=git-raw`
- `echo "" | ipfs dag put --input-enc=dag-json --format=dag-json`

They both don't error or return a CID

[mvdan]

At least on Go 1.17, I can't reproduce the dag-json one:

$ go version
go version go1.17 linux/amd64
$ git checkout v0.9.1; make build
$ ./cmd/ipfs/ipfs --version
ipfs version 0.9.1
$ printf '' | ./cmd/ipfs/ipfs dag put --input-enc=dag-json --format=dag-json
Error: no input parser for "dag-json"
$ printf '\n' | ./cmd/ipfs/ipfs dag put --input-enc=dag-json --format=dag-json
Error: no input parser for "dag-json"
$ git checkout master; make build
$ ./cmd/ipfs/ipfs --version
ipfs version 0.11.0-dev
$ printf '' | ./cmd/ipfs/ipfs dag put --input-enc=dag-json --format=dag-json
Error: EOF
$ printf '\n' | ./cmd/ipfs/ipfs dag put --input-enc=dag-json --format=dag-json
Error: EOF

And it seems like master (701828d) errors correctly. Am I missing something?

The git-raw case is the same story; on 0.9.1 it errors with "no input parser", and on master it correctly fails:

$ ./cmd/ipfs/ipfs --version
ipfs version 0.11.0-dev
$ echo "test" | ./cmd/ipfs/ipfs dag put --input-enc=git-raw
Error: EOF

[mvdan]

For the sake of completeness, I went through the trouble of installing Go 1.16.7, and it has the same results as Go 1.17 above.

[aschmahmann]

@mvdan are you running a daemon or just running the ipfs offline? If you run ipfs daemon and then those commands you should notice the problems.

To be more precise the EOF errors are ignored by the CLI when it is using the HTTP API behind the scenes (e.g. https://github.com/ipfs/go-ipfs-cmds/blob/4ade007405e5d3befb14184290576c63cc43a6a3/http/responseemitter.go#L175).

So these are both giving you EOFs, but EOF doesn't really indicate an error which is why the HTTP API skips it.

[mvdan]

Ah, oops. I guess you can file that under "repro steps unclear" :) I wonder why I'm not getting a clear error along the lines of "IPFS daemon not found".

I'll take another look.

[mvdan]

Thanks, I can reproduce now. TIL that the ipfs CLI can work in "offline mode" as well as "online mode" with a daemon.

I've looked through the code, and I think I agree with Eric. The codec decoder is returning io.EOF when it should be returning io.ErrUnexpectedEOF. The latter is clearly fatal; the former is usually just a "no more input" signal.

It's perhaps wrong for go-ipfs-cmds to ignore io.EOF as a nil error, but changing that will likely break other stuff. Much easier to make the ipld-prime codecs return better errors, which we probably want to do anyway.

[mvdan]

Alright, the upstream fix for dag-json is ipld/go-ipld-prime#241, and for git-raw it's ipfs/go-ipld-git#53. When those are merged, we should be able to just pull master from both of them and be ready for a release. If we need those to also be releases, we can also do that.

I imagine that regression tests for ipfs dag put would be a bit overkill, as I'm already adding tests upstream. But I'm happy to also write some here if necessary.