ipfsconn/ipfshttp: handle cid args passed in url path correctly #392

lanzafame · 2018-04-25T03:01:48Z

The extractCid function was added to enable the extraction of
a cid argument from either the url path or query string.
This puts the proxy behaviour on par with the current IPFS API.
The function does rely on the fact that ipfs-cluster doesn't
intercept any command that has more than one subcommand.
If that changes, this function will have to be updated.

License: MIT
Signed-off-by: Adrian Lanzafame adrianlanzafame92@gmail.com

hsanjuan

Tests fail. I am not sure that the handle is selecting the right thing from ipfs.handlers anymore. We might have to make more clever route matching..

hsanjuan · 2018-04-25T06:19:11Z

ipfsconn/ipfshttp/ipfshttp.go

+
+// extractCid extracts the cid argument from a url.URL, either via
+// the query string parameters or from the url path itself.
+func extractCid(u *url.URL) (string, bool) {


rename to extractArgument, since the argument doesn't have to be a Cid in all cases (even if now it's always one).

hsanjuan · 2018-04-25T06:33:27Z

ipfsconn/ipfshttp/ipfshttp.go

+
+	if len(segs) > 2 {
+		return segs[len(segs)-1], true
+	}


Maybe I'm starting to get what you were saying yesterday..

In general, we have things like:

pin/ls/<arg>

cat/<arg>

but from here we have no way of distinguishing which commands are valid in order to know what part of the url is the argument.

Let's print a warning "You are using an undocumented form of the IPFS API. Consider passing your command arguments with the '?arg=' query parameter".

For the moment it works like this I guess...

Yeah, and after our conversation, I had another look at https://github.com/ipfs/go-ipfs-cmds/blob/master/http/parse.go#L42 and realised that it has access to all the possible subcommands and hence why it is able to distinguish between commands and arguments.

We'll we do know which subcommands we are handling, so that's our list if we want to be strict.

Let's print a warning "You are using an undocumented form of the IPFS API. Consider passing your command arguments with the '?arg=' query parameter".

@hsanjuan should I be returning that in the json response, as it is the client that needs to change?

Also, the way that we have the IPFS-Cluster api documented in the README suggests that pin/ls/<arg> is the more appropriate way to interact with the api, so that should probably get changed as well.

@hsanjuan should I be returning that in the json response, as it is the client that needs to change?

No, just spam the logs

/pin/ls/<arg> that refers to the cluster API.

This is different from the IPFS API.

lanzafame · 2018-04-30T03:04:06Z

ipfsconn/ipfshttp/ipfshttp.go

 		ipfsErrorResponder(w, "Error: bad argument")
 		return
 	}
-	arg := argA[0]
 	_, err := cid.Decode(arg)
 	if err != nil {
 		ipfsErrorResponder(w, "Error parsing CID: "+err.Error())


@hsanjuan So for some reason, the tests that attempt to pin/unpin a bad cid are failing on travis/jenkins (they aren't failing locally and I have tried on two separate computers/OSes). It would appear that on travis/jenkins, this cid.Decode operation is returning an error when passed the test.ErrorCid. Or the extractArgument function is failing...

lanzafame · 2018-04-30T04:13:02Z

@hsanjuan so I haven't gotten to the bottom of this but it appears to be due to travis/jenkins running go1.9 whereas I have been deving with go1.10.1, just trying to figure out which part of is to blame.

coveralls · 2018-04-30T05:10:38Z

Coverage increased (+0.1%) to 67.44% when pulling b50a05f on fix/ipfsconn/api-compat into dbcc5c2 on master.

The extractCid function was added to enable the extraction of a cid argument from either the url path or query string. This puts the proxy behaviour on par with the current IPFS API. The function does rely on the fact that ipfs-cluster doesn't intercept any command that has more than one subcommand. If that changes, this function will have to be updated. License: MIT Signed-off-by: Adrian Lanzafame <adrianlanzafame92@gmail.com>

query args correctly, requiring both a trailing slash and non-trailing slash handle pattern to be defined for the pin and unpin handlers. License: MIT Signed-off-by: Adrian Lanzafame <adrianlanzafame92@gmail.com>

hsanjuan · 2018-04-30T07:51:29Z

@lanzafame this is ready on your side right?

lanzafame · 2018-04-30T07:52:27Z

I just want to squash done commits otherwise yes.

On Mon., 30 Apr. 2018, 17:51 Hector Sanjuan, ***@***.***> wrote: @lanzafame <https://github.com/lanzafame> this is ready on your side right? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#392 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFpnaOwrnj-wUh3r0b-5DJrH5McqBOcjks5ttsKCgaJpZM4Titj5> .

-- Adrian Lanzafame m: 0435419266 e: adrianlanzafame92@gmail.com gh: github.com/lanzafame

lanzafame · 2018-04-30T07:54:17Z

s/done/some On Mon., 30 Apr. 2018, 17:52 Adrian Lanzafame, <adrianlanzafame92@gmail.com> wrote:

I just want to squash done commits otherwise yes. On Mon., 30 Apr. 2018, 17:51 Hector Sanjuan, ***@***.***> wrote: > @lanzafame <https://github.com/lanzafame> this is ready on your side > right? > > — > You are receiving this because you were mentioned. > > > Reply to this email directly, view it on GitHub > <#392 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AFpnaOwrnj-wUh3r0b-5DJrH5McqBOcjks5ttsKCgaJpZM4Titj5> > . > -- Adrian Lanzafame m: 0435419266 e: ***@***.*** gh: github.com/lanzafame

-- Adrian Lanzafame m: 0435419266 e: adrianlanzafame92@gmail.com gh: github.com/lanzafame

lanzafame · 2018-05-01T02:47:58Z

@hsanjuan Sorry, this PR is fine regarding commits. I was thinking of the pintracker revamp PR.
This one is good to go.

hsanjuan · 2018-05-01T08:37:13Z

Hehe yeah I was thinking so :)

lanzafame requested a review from hsanjuan April 25, 2018 03:01

ghost assigned lanzafame Apr 25, 2018

ghost added the status/in-progress In progress label Apr 25, 2018

lanzafame force-pushed the fix/ipfsconn/api-compat branch from c9c22b8 to 3b636a3 Compare April 25, 2018 03:04

hsanjuan requested changes Apr 25, 2018

View reviewed changes

lanzafame force-pushed the fix/ipfsconn/api-compat branch from 3b636a3 to 267c7ba Compare April 27, 2018 07:58

lanzafame assigned lanzafame and unassigned lanzafame Apr 29, 2018

lanzafame force-pushed the fix/ipfsconn/api-compat branch 4 times, most recently from e6f0d58 to d596361 Compare April 30, 2018 00:55

lanzafame commented Apr 30, 2018

View reviewed changes

lanzafame force-pushed the fix/ipfsconn/api-compat branch from 5b1e2f4 to fa4fe5c Compare April 30, 2018 04:58

lanzafame added 2 commits April 30, 2018 15:21

ipfsconn/ipfshttp: go1.9 ServeMux doesn't redirect

b50a05f

query args correctly, requiring both a trailing slash and non-trailing slash handle pattern to be defined for the pin and unpin handlers. License: MIT Signed-off-by: Adrian Lanzafame <adrianlanzafame92@gmail.com>

lanzafame force-pushed the fix/ipfsconn/api-compat branch from fa4fe5c to b50a05f Compare April 30, 2018 05:21

hsanjuan approved these changes Apr 30, 2018

View reviewed changes

hsanjuan added the RFM Ready for Merge label Apr 30, 2018

hsanjuan merged commit b01ac8f into master May 1, 2018

ghost removed the status/in-progress In progress label May 1, 2018

hsanjuan deleted the fix/ipfsconn/api-compat branch May 1, 2018 08:37

hsanjuan mentioned this pull request May 1, 2018

Removing or listing pins gives inconsistent results between cluster CLI and proxied API. #380

Closed

hsanjuan mentioned this pull request May 1, 2018

Release 0.4.0 #372

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ipfsconn/ipfshttp: handle cid args passed in url path correctly #392

ipfsconn/ipfshttp: handle cid args passed in url path correctly #392

lanzafame commented Apr 25, 2018 •

edited

hsanjuan left a comment

hsanjuan Apr 25, 2018

hsanjuan Apr 25, 2018

lanzafame Apr 26, 2018

hsanjuan Apr 26, 2018

lanzafame Apr 26, 2018

hsanjuan Apr 27, 2018 •

edited

lanzafame Apr 30, 2018 •

edited

lanzafame commented Apr 30, 2018

coveralls commented Apr 30, 2018 •

edited

hsanjuan commented Apr 30, 2018

lanzafame commented Apr 30, 2018 via email

lanzafame commented Apr 30, 2018 via email

lanzafame commented May 1, 2018

hsanjuan commented May 1, 2018

ipfsconn/ipfshttp: handle cid args passed in url path correctly #392

ipfsconn/ipfshttp: handle cid args passed in url path correctly #392

Conversation

lanzafame commented Apr 25, 2018 • edited

hsanjuan left a comment

Choose a reason for hiding this comment

hsanjuan Apr 25, 2018

Choose a reason for hiding this comment

hsanjuan Apr 25, 2018

Choose a reason for hiding this comment

lanzafame Apr 26, 2018

Choose a reason for hiding this comment

hsanjuan Apr 26, 2018

Choose a reason for hiding this comment

lanzafame Apr 26, 2018

Choose a reason for hiding this comment

hsanjuan Apr 27, 2018 • edited

Choose a reason for hiding this comment

lanzafame Apr 30, 2018 • edited

Choose a reason for hiding this comment

lanzafame commented Apr 30, 2018

coveralls commented Apr 30, 2018 • edited

hsanjuan commented Apr 30, 2018

lanzafame commented Apr 30, 2018 via email

lanzafame commented Apr 30, 2018 via email

lanzafame commented May 1, 2018

hsanjuan commented May 1, 2018

lanzafame commented Apr 25, 2018 •

edited

hsanjuan Apr 27, 2018 •

edited

lanzafame Apr 30, 2018 •

edited

coveralls commented Apr 30, 2018 •

edited