Redirect on 307, 308, 303

[djmadeira]

Fixes #291.

Open questions:

• Should there be an option for whether GET or HEAD are used to follow up a 303?
• I have written this for clarity over performance. LMK if you'd prefer it be more performance-oriented.

TODO:

• Update README

[djmadeira]

Fails on node4, which shouldn't be a blocker for changes to 7.0.0 release.

[sindresorhus]

This would be a good use for Set(). You could then use .has() instead of .includes() on line 50.

[sindresorhus]

We still support Node.js 4. Just do .includes(opts.method) => .indexOf(opts.method) !== -1

[sindresorhus]

Should there be an option for whether GET or HEAD are used to follow up a 303?

From what I could tell from https://en.m.wikipedia.org/wiki/HTTP_303 it should use the original method, unless it's POST, then it should be changed to GET. Make sure it works like this here.

[djmadeira]

From RFC 7231:

The 303 (See Other) status code indicates that the server is
redirecting the user agent to a different resource, as indicated by a
URI in the Location header field, which is intended to provide an
indirect response to the original request. A user agent can perform
a retrieval request targeting that URI (a GET or HEAD request if
using HTTP), which might also be redirected, and present the eventual
result as an answer to the original request. Note that the new URI
in the Location header field is not considered equivalent to the
effective request URI.

This status code is applicable to any HTTP method. It is primarily
used to allow the output of a POST action to redirect the user agent
to a selected resource, since doing so provides the information
corresponding to the POST response in a form that can be separately
identified, bookmarked, and cached, independent of the original
request.

A 303 response to a GET request indicates that the origin server does
not have a representation of the target resource that can be
transferred by the server over HTTP. However, the Location field
value refers to a resource that is descriptive of the target
resource, such that making a retrieval request on that other resource
might result in a representation that is useful to recipients without
implying that it represents the original target resource. Note that
answers to the questions of what can be represented, what
representations are adequate, and what might be a useful description
are outside the scope of HTTP.

My reading:

• Can apply to any request method, including GET
• When in response to GET, means the server can't represent the entity over HTTP, but something descriptive of the entity can be found at the Location header

Based on this, maybe we should have some kind of error response specific to receiving a 303 in response to a GET? Since clients may need to make a decision about how to proceed.

While we're on the topic, according to the spec, we should probably do something similar in response to a 300:

The 300 (Multiple Choices) status code indicates that the target
resource has more than one representation, each with its own more
specific identifier, and information about the alternatives is being
provided so that the user (or user agent) can select a preferred
representation by redirecting its request to one or more of those
identifiers. In other words, the server desires that the user agent
engage in reactive negotiation to select the most appropriate
representation(s) for its needs (Section 3.4).

If the server has a preferred choice, the server SHOULD generate a
Location header field containing a preferred choice's URI reference.
The user agent MAY use the Location field value for automatic
redirection.

For request methods other than HEAD, the server SHOULD generate a
payload in the 300 response containing a list of representation
metadata and URI reference(s) from which the user or user agent can
choose the one most preferred. The user agent MAY make a selection
from that list automatically if it understands the provided media
type. A specific format for automatic selection is not defined by
this specification because HTTP tries to remain orthogonal to the
definition of its payloads. In practice, the representation is
provided in some easily parsed format believed to be acceptable to
the user agent, as determined by shared design or content
negotiation, or in some commonly accepted hypertext format.

[alextes]

I would always use GET. People can create an issue if there is a need for a GET or HEAD option.

EDIT: sindresorhus if I remember correctly it's for any method. One can imagine deleting a resource that's maybe a child to a parent and thus the server redirects to the parent resource for the result.

[djmadeira]

Actually not sure what I would update in the README, except maybe the surprising behavior of changing from POST/PUT/DELETE to GET on a 303 response? Seems like a case for a changelog entry if one existed.

[sindresorhus]

except maybe the surprising behavior of changing from POST/PUT/DELETE to GET on a 303 response

I think that would be enough. Can you add that to the readme?

Seems like a case for a changelog entry if one existed.

It does: https://github.com/sindresorhus/got/releases

While we're on the topic, according to the spec, we should probably do something similar in response to a 300:

I guess we could redirect then too if a location header exists. If not, it's up to the user to handle it.

[djmadeira]

@sindresorhus updated README. 300 redirects are already followed; my question was whether, since the spec describes a 300 response as containing multiple choices, we should throw an error in this case so the consumer has a chance to decide which resource to request next.

[floatdrop]

lgtm

[floatdrop]

Regarding multiple choices in redirects – I never met this in real life. I would take location header and go with it, until issues comes up.

[floatdrop]

@djmadeira 👍