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

ipfs --encoding=json ls not returning JSON #7050

Open
dokterbob opened this issue Mar 28, 2020 · 5 comments
Open

ipfs --encoding=json ls not returning JSON #7050

dokterbob opened this issue Mar 28, 2020 · 5 comments
Labels
help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@dokterbob
Copy link
Contributor

Version information:

go-ipfs version: 0.4.23-6ce9a35
Repo version: 7
System version: amd64/darwin
Golang version: go1.13.7

Description:

The output of ipfs --encoding=json ls is expected to return JSON, as with other commands. Instead, the output is the same as without the encoding parameter.

Example:

$ ipfs ls /ipfs/QmQy8FTQCdJGtNHg92pc4B6F4cjnuSdrFx9gdRmkrE1rVF
QmWyDJmrr6cRwEpTF2VGhWDi4uytrDHT8S5BptVdkbhjpv 6060 README.md
$ ipfs --encoding=json ls /ipfs/QmQy8FTQCdJGtNHg92pc4B6F4cjnuSdrFx9gdRmkrE1rVF
QmWyDJmrr6cRwEpTF2VGhWDi4uytrDHT8S5BptVdkbhjpv 6060 README.md

Expected output similar to HTTP API call:

curl "http://localhost:5001/api/v0/ls?arg=/ipfs/QmQy8FTQCdJGtNHg92pc4B6F4cjnuSdrFx9gdRmkrE1rVF" | jq
{
  "Objects": [
    {
      "Hash": "/ipfs/QmQy8FTQCdJGtNHg92pc4B6F4cjnuSdrFx9gdRmkrE1rVF",
      "Links": [
        {
          "Name": "README.md",
          "Hash": "QmWyDJmrr6cRwEpTF2VGhWDi4uytrDHT8S5BptVdkbhjpv",
          "Size": 6060,
          "Type": 2,
          "Target": ""
        }
      ]
    }
  ]
}
@dokterbob dokterbob added the kind/bug A bug in existing code (including security flaws) label Mar 28, 2020
@hsanjuan hsanjuan added help wanted Seeking public contribution on this issue P2 Medium: Good to have, but can wait until someone steps up labels Mar 28, 2020
@hsanjuan
Copy link
Contributor

Linking to a similar one #5594. I wonder if this an easy fix or there is more to it though.

@Stebalien
Copy link
Member

I believe the solution is to detect that (a) the output is text and (b) the target is a terminal in PostRun. Then, just forward results instead of rendering them.

@Stebalien
Copy link
Member

Even better, introduce a Terminal PostRun type and only run it under those conditions.

@hsanjuan
Copy link
Contributor

hsanjuan commented Apr 1, 2020

But @Stebalien , why do we have to detect anything, if I'm explicitally asking for --enc=json, shouldn't it just use the json encoder with that option?

It sounds like it should still use text when the target is not a terminal, but it should use whatever the --enc= flag says.

@Stebalien
Copy link
Member

The problem is that we're abusing PostRun. It's designed to post-process results on the client (read results from the response, re-emit them to the next response emitter). Take a look at the ping command.

However, we're using it to write to the console. We should be using the text encoder but:

  1. The text encoder is invoked once per event. This makes it hard to initialize/finalize the output.
  2. The text encoder will be run on the server if the user makes a curl request. That's usually not what we want.

That's why I'm saying we should consider having some kind of Terminal post-run command. Or maybe a new encoder? Or even an entirely new Display function ipfs/go-ipfs-cmds#115?

jbouwman pushed a commit to jbouwman/go-ipfs-cmds that referenced this issue Aug 19, 2021
Several open issues mention problems with interaction of the
global `--encoding=` flag and the Encoders and PostRun fields of
command structs. This branch contains experimental refactors that
explore approaches to consistent command execution patterns across
offline, online and http modes.

Specific tickets:

- ipfs/kubo#7050 json encoding for `ls`
- ipfs/kubo#1121 json encoding for `add`
- ipfs/kubo#5594 json encoding for `stats bw`
- ipfs#115 postrun design

Possibly related:

- ipfs/kubo#6640 global flags on subcommands

Incomplete PRs:

- ipfs/kubo#5620 json for 'stat'
jbouwman added a commit to jbouwman/go-ipfs-cmds that referenced this issue Aug 21, 2021
Following the approach described in ipfs#115, define a new method signature on `Command` that supports full processing of the `Response` object when text encoding is requested.

Add an encoding check and dispatch to DisplayCLI in local, http client, and http handler code paths.

Unblocks resolution of `encoding` option processing in multiple go-ipfs issues.

- ipfs/kubo#7050 json encoding for `ls`
- ipfs/kubo#1121 json encoding for `add`
- ipfs/kubo#5594 json encoding for `stats bw`
jbouwman added a commit to jbouwman/go-ipfs-cmds that referenced this issue Sep 8, 2021
Following the approach described in ipfs#115, define a new method signature on `Command` that supports full processing of the `Response` object when text encoding is requested.

Add an encoding check and dispatch to DisplayCLI in local, http client, and http handler code paths.

Unblocks resolution of `encoding` option processing in multiple go-ipfs issues.

- ipfs/kubo#7050 json encoding for `ls`
- ipfs/kubo#1121 json encoding for `add`
- ipfs/kubo#5594 json encoding for `stats bw`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

3 participants