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

Extra newline at end of "dag get" result #3503

Closed
ianopolous opened this issue Dec 13, 2016 · 13 comments
Closed

Extra newline at end of "dag get" result #3503

ianopolous opened this issue Dec 13, 2016 · 13 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked

Comments

@ianopolous
Copy link
Member

Version information:

go-ipfs version: 0.4.5-dev-4cb236c
Repo version: 4
System version: amd64/linux
Golang version: go1.7.1

Type:

Bug

Priority:

P1 (for me)

Description:

Using either the cli or http api the dag get command seems to add an extra newline 0x0A on the end of the raw data.

To reproduce using cli:

echo '{"data":1234}' | ipfs dag put -f cbor

then

ipfs dag get zdpuAs3whHmb9T1NkHSLGF45ykcKrEBxSLiEx6YpLzmKbQLEB | wc -c

should give 13, but gives 14, and the extra byte on the end is a newline

The same JSON through the http api results in the same extra byte. The JSON is not special, similar JSON with more/different elements does the same.

@Kubuxu
Copy link
Member

Kubuxu commented Dec 14, 2016

JSON isn't a stable format, we have no way of knowing was was the initial format of the json you have passed in.

Also you are passing it in with a new line, but doing printf "%s" '{"data":1234}' won't change anything.
The formatting of the output JSON format is arbitrary if you store it as pure data.

@ianopolous
Copy link
Member Author

ipfs dag put --help says that the default input format is JSON.

@djdv
Copy link
Contributor

djdv commented Dec 14, 2016

I think Kubuxu is saying that echo is outputting your string but also a newline, so what you meant to pass in and what is actually being passing in are different. printf will print the string and not add a newline.
untitled

@Kubuxu
Copy link
Member

Kubuxu commented Dec 14, 2016

ipfs dag put --help says that the default input format is JSON.

Yes, input format is JSON but storage format is CBOR, which is stable format.

JSON isn't a stable format, we have no way of knowing was was the initial format of the json you have passed in.

There are infinitely many ways you can store same data in JSON ({"data":1234}, {"data":1234 }, {"data": 1234}, and so on) but from the usability standpoint it is more important that the same information has the same address.

If you want to store the white-space information, use plain ipfs add which doesn't interpret data your are passing it.

@ianopolous
Copy link
Member Author

@djdv I tried the printf suggestion, but, as Jakub predicted, it gives the same result.

@Kubuxu I get that JSON -> CBOR is many to one, and you need some canonical format. What I don't get is why that canonical format has a newline outside a minimal form of the json. As a consumer of the api, even one who is well versed in other aspects of IPFS this is surprising to me. (Also, I originally submitted this after discussing it with @whyrusleeping)

@whyrusleeping
Copy link
Member

Yeah, the primary issue here isnt that the input object differs from the output object, the problem is that theres a newline after the json blob for some reason.

@momack2 momack2 added this to Inbox in ipfs/go-ipfs May 9, 2019
@jacobheun jacobheun added kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked labels May 29, 2020
@jacobheun
Copy link
Contributor

Verified this is still an issue in 0.5

@ianopolous ianopolous mentioned this issue May 14, 2021
71 tasks
@lidel
Copy link
Member

lidel commented Jul 2, 2021

For the record, this is still an issue in 0.9.0.
Adding to maintenance board.

@lidel lidel added this to Weekly Candidates in Maintenance Priorities - Go Jul 2, 2021
@aschmahmann
Copy link
Contributor

This may get resolved in the refactoring of dag get to support go-ipld-prime (e.g. #8171). For now let's just add a sharness test that expects to fail and we can flip the switch to expecting success in the rewriting PR to see if the issue persists.

@aschmahmann aschmahmann assigned masih and unassigned mvdan Jul 19, 2021
@aschmahmann aschmahmann added the P1 High: Likely tackled by core team if no one steps up label Jul 19, 2021
masih added a commit that referenced this issue Jul 20, 2021
Write a `sharness` test that expects failure documenting the issue
raised in #3503. The issue may get resolved in the refactoring of
`ipfs dag get` command to support go-ipld-prime (e.g. #8171). For now
add a `sharness` test that expects failure. We will then flip the
expectation in the success in the rewriting PR to check if #3503 gets
resolved.

Relates to:
- #3503
@masih
Copy link
Member

masih commented Jul 20, 2021

This may get resolved in the refactoring of dag get to support go-ipld-prime (e.g. #8171). For now let's just add a sharness test that expects to fail and we can flip the switch to expecting success in the rewriting PR to see if the issue persists.

Implemented here: #8280

Stebalien added a commit that referenced this issue Jul 21, 2021
@masih
Copy link
Member

masih commented Jul 22, 2021

@aschmahmann I can confirm #7995 fixes this issue.

@aschmahmann
Copy link
Contributor

Great, we'll close this issue out once it's been incorporated into master.

@aschmahmann aschmahmann moved this from Weekly Candidates to In Progress in Maintenance Priorities - Go Jul 30, 2021
aschmahmann added a commit that referenced this issue Aug 17, 2021
* feat: switch to using go-ipld-prime for codecs, path resolution, and the `dag put/get` commands
* fix: `dag put/get` not roundtripping due to an extra new line being added (#3503)

More detailed information is in the CHANGELOG.md file. Very high level:
* IPLD codecs (and their plugins) must use go-ipld-prime
* Added support for the dag-json codec
* `dag get/put` use IPLD codec names from the multicodec table
* `dag get` defaults to dag-json output instead of json, but may output with other codecs
* Data model pathing can be achieved using the /ipld prefix. For example, you can use `/ipld/QmFoo/Links/0/Hash` to traverse through a DagPB node
* With `dag get/put` the DagPB field names have been changed to match the ones in the protobuf listed in the specification

Co-authored-by: hannahhoward <hannah@hannahhoward.net>
Co-authored-by: Daniel Martí <mvdan@mvdan.cc>
Co-authored-by: acruikshank <acruikshank@example.com>
Co-authored-by: Steven Allen <steven@stebalien.com>
Co-authored-by: Will Scott <will.scott@protocol.ai>
Co-authored-by: Will Scott <will@cypherpunk.email>
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>
Co-authored-by: Eric Myhre <hash@exultant.us>
@BigLep
Copy link
Contributor

BigLep commented Aug 20, 2021

Closing since it's in RC

@BigLep BigLep closed this as completed Aug 20, 2021
Maintenance Priorities - Go automation moved this from In Progress to Done Aug 20, 2021
@aschmahmann aschmahmann mentioned this issue Aug 23, 2021
62 tasks
hannahhoward added a commit to filecoin-project/kubo-api-client that referenced this issue Jun 19, 2023
* feat: switch to using go-ipld-prime for codecs, path resolution, and the `dag put/get` commands
* fix: `dag put/get` not roundtripping due to an extra new line being added (ipfs/kubo#3503)

More detailed information is in the CHANGELOG.md file. Very high level:
* IPLD codecs (and their plugins) must use go-ipld-prime
* Added support for the dag-json codec
* `dag get/put` use IPLD codec names from the multicodec table
* `dag get` defaults to dag-json output instead of json, but may output with other codecs
* Data model pathing can be achieved using the /ipld prefix. For example, you can use `/ipld/QmFoo/Links/0/Hash` to traverse through a DagPB node
* With `dag get/put` the DagPB field names have been changed to match the ones in the protobuf listed in the specification

Co-authored-by: hannahhoward <hannah@hannahhoward.net>
Co-authored-by: Daniel Martí <mvdan@mvdan.cc>
Co-authored-by: acruikshank <acruikshank@example.com>
Co-authored-by: Steven Allen <steven@stebalien.com>
Co-authored-by: Will Scott <will.scott@protocol.ai>
Co-authored-by: Will Scott <will@cypherpunk.email>
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>
Co-authored-by: Eric Myhre <hash@exultant.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked
Projects
No open projects
Development

No branches or pull requests

10 participants