search

feross / simple-get

Simplest way to make http get requests. Supports HTTPS, redirects, gzip/deflate, streams in < 100 lines
MIT License
401 stars 50 forks source link

Do not follow Location headers for POSTs #46

Open mcollina opened 5 years ago

mcollina commented 5 years ago

Currently simple-get will send a duplicated POST request if the response has a location header. I think this is not correct because POST requests are not idempotent. Moreover this is very common in REST applications, return a 204 status code with a Location header for a resource that should be accessed via GET (not POST).

I'm happy to file a PR if you agree that this should be fixed.

bendrucker commented 5 years ago

Shouldn't this only be happening with a 3xx status code?

https://github.com/feross/simple-get/blob/master/index.js#L48

While POSTs aren't idempotent, it should be safe to send another request when the server specifically says so. It's definitely reasonable/common to send back a Location header with the URL for the new resource in response to a POST. GitHub does it, for example.

mcollina commented 5 years ago

The problem is for 3xx status codes. According to MDN, https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/303:

This response code is usually sent back as a result of PUT or POST. The method used to display this redirected page is always GET.

bendrucker commented 5 years ago

Seems like that's 303-specific. 301 and 302 are not recommending for GET/HEAD only, which I'd agree with. I can't say I've ever seen an API that sends back 3xx codes for writes. Do you have an example of where this is happening?

mcollina commented 5 years ago

The issue is an example of how unexpected it is to see a POST request repeated. I think following redirects should be a GET-only behavior.

POST requests are non-repeateable actions, as such they should not be repeated and the control should return to the user.

bendrucker commented 5 years ago

In a semantically correct API, yes, POSTs are not idempotent. But it's also pretty non-standard to send back a 301 for a POST. Someone could be relying on redirect triggering a second request.

Passing back control is a probably a good option for everyone since someone depending on a redirect can do so themselves.

I'm interested in how other user agents (browsers, other node http clients) handle this. I suspect browsers might have some form of cross origin restriction for following non GET/HEAD redirects if they follow them at all.

It is simple-get so I can get behind not redirecting writes for simplicity.

mcollina commented 5 years ago

https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections is a good resource that lists the behavior of all status code.

I think the simpler implementation is to not follow redirects for anything but HEAD or GET requests, as they are the only one guaranteed idempotent. We could enable some HTTP code to redirect on POST etc, as some are definitely safe.