Go-ipfs: NDJSON API endpoins should use application/x-ndjson Content-Type header

[mitar]

Version information:

go-ipfs version: 0.4.5-
Repo version: 5
System version: amd64/darwin
Golang version: go1.7.4

Type: Bug

Priority: P2

Description:

Compare:

curl -D - "http://localhost:5001/api/v0/log/tail"
HTTP/1.1 200 OK
Access-Control-Allow-Headers: X-Stream-Output, X-Chunked-Output, X-Content-Length
Access-Control-Expose-Headers: X-Stream-Output, X-Chunked-Output, X-Content-Length
Content-Type: text/plain
Server: go-ipfs/0.4.5
Trailer: X-Stream-Error
Vary: Origin
X-Stream-Output: 1
Date: Wed, 01 Mar 2017 10:14:45 GMT
Transfer-Encoding: chunked

curl -D - "http://localhost:5001/api/v0/pubsub/sub?arg=<topic>&discover=false"
HTTP/1.1 200 OK
Access-Control-Allow-Headers: X-Stream-Output, X-Chunked-Output, X-Content-Length
Access-Control-Expose-Headers: X-Stream-Output, X-Chunked-Output, X-Content-Length
Content-Type: application/json
Server: go-ipfs/0.4.5
Trailer: X-Stream-Error
Vary: Origin
X-Chunked-Output: 1
Date: Wed, 01 Mar 2017 10:14:53 GMT
Transfer-Encoding: chunked

Not only they are different, they are both wrong as proper content type is application/x-ndjson.

Edited by Kubuxu

[mitar]

Hm, it seems this is handled specially. In contrast then with log, where it is parsed here instead.

[Kubuxu]

Is the curl -D - "http://localhost:5001/api/v0/pubsub/sub?arg=<topic>&discover=false" returning valid ndjson?

[mitar]

It is returning valid JSONs per newline. It is just confusing that log returns a different content type than pubsub.

[Kubuxu]

We should probably do the reverse then, change the response type of the log tail to json.

[Kubuxu]

Really the content type should be application/x-ndjson.

[mitar]

Yea. Then also this could be simplified to simply passing callback to send.

[mitar]

Really the content type should be application/x-ndjson.

True, because my browser extension complains now when it tries to parse application/json.

[Kubuxu]

But it is breaking change to the API, so we have to be really careful with it.

[Kubuxu]

@mitar I've edited the title and the main post. Please see if it is correct.

[mitar]

Looks good. Thanks!

[Kubuxu]

@whyrusleeping @diasdavid @dignifiedquire please take a look at this issue. More we wait for this to happen the harder it will to migrate. I am open to various resolutions. It again reopens problem of API versioning. We claim to be v0 but we treat it as v1.

[dignifiedquire]

I'm all for valid content headers.

[hsanjuan]

Yeah put valid content headers. This is broken for people that look at that header so I don't think they will complain. And for those that don't look at it it should be harmless. Do we need to update any spec?

[whyrusleeping]

Yeah, i'm fine putting in valid content types here.

[Kubuxu]

It will break js-ipfs-api, afaik.

[mitar]

But a change is simple and mentioned above. Just remove extra ndjson parse.

[whyrusleeping]

@Kubuxu eh, unsure. I think they can fix those things to not rely on the content type before we make this change so that it doesnt break when we fix it.

cc @diasdavid @dignifiedquire

[dignifiedquire]

Just make the PR, I can fix the api before a release happens.

[whyrusleeping]

@dignifiedquire You should be able to fix it so that it works for current behaviour and with the changed content type (making this change on the go side requires no changes to our api client)