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

Following a 303 after a PUT requests uses PUT instead of GET #5237

Closed
PofMagicfingers opened this issue Apr 14, 2020 · 19 comments
Closed

Following a 303 after a PUT requests uses PUT instead of GET #5237

PofMagicfingers opened this issue Apr 14, 2020 · 19 comments
Labels

Comments

@PofMagicfingers
Copy link

@PofMagicfingers PofMagicfingers commented Apr 14, 2020

I know you had issues opened with this kind of title, and people were using -X PUT to force PUT, but not this time.

I did this

touch somefile.txt
curl -T 'somefile.txt' -v -L https://httpstat.us/301 > /dev/null

( as adviced here : #578 (comment) )

I expected the following

curl should send somefile.txt with a PUT request, then follow the 303 redirect with a GET request. Instead it uses the PUT verb.

Issue does not occur when server returns a 301 or 302 redirect.

If it isn't the right way to send a put request and follow a 303 response, I'm sorry to have bothered you, and would be glad to know how to do that ! 🙂

Output of the command

(some tls and progress bar logs have been redacted for readability)

*   Trying 23.99.0.12:443...
* Connected to httpstat.us (23.99.0.12) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
> PUT /301 HTTP/1.1
> Host: httpstat.us
> User-Agent: curl/7.69.1
> Accept: */*
> Content-Length: 0
>
< HTTP/1.1 301 Moved Permanently
< Cache-Control: private
< Content-Length: 21
< Content-Type: text/plain; charset=utf-8
< Location: https://httpstat.us
< Server: Microsoft-IIS/10.0
< X-AspNetMvc-Version: 5.1
< Access-Control-Allow-Origin: *
< X-AspNet-Version: 4.0.30319
< Request-Context: appId=cid-v1:7585021b-2db7-4da6-abff-2cf23005f0a9
< Access-Control-Expose-Headers: Request-Context
< X-Powered-By: ASP.NET
< Set-Cookie: ARRAffinity=dc9bb7185987c0d65790987c33534f3a4e8956d9544852ccf290ce45876d7754;Path=/;HttpOnly;Domain=httpstat.us
< Date: Tue, 14 Apr 2020 17:16:50 GMT
<
* Ignoring the response-body
* Connection #0 to host httpstat.us left intact
* Issue another request to this URL: 'https://httpstat.us/'
* Found bundle for host httpstat.us: 0x55e7f74485c0 [serially]
* Can not multiplex, even if we wanted to!
* Re-using existing connection! (#0) with host httpstat.us
* Connected to httpstat.us (23.99.0.12) port 443 (#0)
> PUT / HTTP/1.1
> Host: httpstat.us
> User-Agent: curl/7.69.1
> Accept: */*
> Content-Length: 0
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Cache-Control: private
< Content-Length: 8553
< Content-Type: text/html; charset=utf-8
< Server: Microsoft-IIS/10.0
< X-AspNetMvc-Version: 5.1
< Access-Control-Allow-Origin: *
< X-AspNet-Version: 4.0.30319
< Request-Context: appId=cid-v1:7585021b-2db7-4da6-abff-2cf23005f0a9
< Access-Control-Expose-Headers: Request-Context
< X-Powered-By: ASP.NET
< Set-Cookie: ARRAffinity=dc9bb7185987c0d65790987c33534f3a4e8956d9544852ccf290ce45876d7754;Path=/;HttpOnly;Domain=httpstat.us
< Date: Tue, 14 Apr 2020 17:16:50 GMT
<
* Connection #0 to host httpstat.us left intact

curl/libcurl version

curl 7.69.1 (x86_64-pc-linux-gnu) libcurl/7.69.1 OpenSSL/1.1.1f zlib/1.2.11 libidn2/2.3.0 libpsl/0.21.0 (+libidn2/2.2.0) libssh2/1.9.0 nghttp2/1.40.0
Release-Date: 2020-03-11
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS GSS-API HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL TLS-SRP UnixSockets

operating system

Linux arceus 5.4.30-1-MANJARO #1 SMP PREEMPT Thu Apr 2 17:31:45 UTC 2020 x86_64 GNU/Linux

@bagder bagder added the HTTP label Apr 14, 2020
@jay
Copy link
Member

@jay jay commented Apr 14, 2020

curl should send somefile.txt with a PUT request, then follow the 303 redirect with a GET request. Instead it uses the PUT verb.

I'm pretty sure that applies only to POST, @jzakrzewski's comment seems wrong. libcurl has CURLOPT_POSTREDIR but there is no similar option for redirection on PUT.

edit: See 8b7fff3, #4859 (comment)

@jay jay added the not-a-bug label Apr 14, 2020
@jzakrzewski
Copy link
Contributor

@jzakrzewski jzakrzewski commented Apr 15, 2020

Yes, I don't see any reference to changing the method to GET on PUT redirects. So I stand corrected on that one.
However what about this wording:

If the 301 status code is received in response to a request other than GET or HEAD, the user agent MUST NOT automatically redirect the request unless it can be confirmed by the user, since this might change the conditions under which the request was issued.

For me it means the redirects must work the same for POST and PUT.

@PofMagicfingers
Copy link
Author

@PofMagicfingers PofMagicfingers commented Apr 15, 2020

According to most resources and documentation, a user agent following a 303 redirect must use the HEAD or GET verb :

Wikipedia:

 If a server responds to a POST or other non-idempotent request with a 303 See Other response and a value for the location header, the client is expected to obtain the resource mentioned in the location header using the GET method; to trigger a request to the target resource using the same method, the server is expected to provide a 307 Temporary Redirect response

https://en.m.wikipedia.org/wiki/HTTP_303

MDN says :

The method used to display this redirected page is always GET

https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/303

And of the official specifications states :

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

https://tools.ietf.org/html/rfc7231#section-6.4.4

(sorry for the short reply and lack of formating, I'm on mobile)

@michael-o
Copy link
Member

@michael-o michael-o commented Apr 15, 2020

I don't see anything in rfc7231#section-6.4.4 which says this applies to anything else, but POST.

@jzakrzewski
Copy link
Contributor

@jzakrzewski jzakrzewski commented Apr 15, 2020

For me it's a bit unclear. Yes, section 6.4 only names POST specifically but there's also:

Automatic redirection needs to done with
care for methods not known to be safe, as defined in Section 4.2.1,
since the user might not wish to redirect an unsafe request.

And in 4.2.1:

Of the request methods defined by this specification, the GET, HEAD,
OPTIONS, and TRACE methods are defined to be safe.

From the other hand in 4.2.2:

Of the request methods
defined by this specification, PUT, DELETE, and safe request methods
are idempotent.

So it should at least be safe to issue PUT after redirection.

@michael-o
Copy link
Member

@michael-o michael-o commented Apr 15, 2020

When it doubt, file an issue: https://github.com/httpwg/http-core.

Here is a similiar one, applies to POST only: apache/httpcomponents-client@53e1725

@jzakrzewski
Copy link
Contributor

@jzakrzewski jzakrzewski commented Apr 15, 2020

Oh, I think I've found the key phrase.
In 4.3.4:

If the origin server will not make the requested PUT state change to
the target resource and instead wishes to have it applied to a
different resource, such as when the resource has been moved to a
different URI, then the origin server MUST send an appropriate 3xx
(Redirection) response; the user agent MAY then make its own decision
regarding whether or not to redirect the request.

So I guess curl's behaviour is at least correct according to this specs. Even if a bit surprising for the user.

@PofMagicfingers
Copy link
Author

@PofMagicfingers PofMagicfingers commented Apr 15, 2020

I don't see anything in rfc7231#section-6.4.4 which says this applies to anything else, but POST.

Section 6.4.4 is saying :

A user agent can perform
a retrieval request targeting that URI (a GET or HEAD request if
using HTTP)

And few lines below :

This status code is applicable to any HTTP method

About this :

From the other hand in 4.2.2:

Of the request methods
defined by this specification, PUT, DELETE, and safe request methods
are idempotent.

So it should at least be safe to issue PUT after redirection.

I don't get you point, it is probably safe to issue a PUT request with the same parameters to the same URL, but it doesn't means anything about using the PUT verb to issue a redirect to another URI.

@jzakrzewski
Copy link
Contributor

@jzakrzewski jzakrzewski commented Apr 15, 2020

I don't get you point, it is probably safe to issue a PUT request with the same parameters to the same URL, but it doesn't means anything about using the PUT verb to issue a redirect to another URI.

My point is that PUT is not among safe methods what would imply the method should be changed to GET on redirect but being idempotent not changing the method is at least safe, so it's not a dangerous bug (if a bug at all).

In any case my next comment citing section 4.3.4 seems to make it clear that not changing the method is in fact an allowed behaviour (unless I don't understand / miss something).

@PofMagicfingers
Copy link
Author

@PofMagicfingers PofMagicfingers commented Apr 15, 2020

I really don't see anything about changing the verb or not in 4.3.4, it only states the server MUST send a 3xx redirect if the resource has changed URI and the user agent MAY follow or not that redirection. But nothing is said about verbs.

In other hand, there is a specific 307/308 redirect status code designed to redirect without changing verb in any case.

I'm really confused about where is the ambiguity here as the spec clearly states in 6.4.4 :

A user agent can perform a retrieval request targeting that URI (a GET or HEAD request if using HTTP)

[...]

This status code is applicable to any HTTP method

Maybe I'm getting this wrong but it seems to be clear that the specs are saying : a 303 redirect can be followed by user agent, with a GET or HEAD request.

I will probably open an issue in http-core send an email to their mailing list, to get the official answer

@PofMagicfingers
Copy link
Author

@PofMagicfingers PofMagicfingers commented Apr 15, 2020

I sent an email regarding this issue to their mailing list. I'll let you know if I get any answers

By the way I also stumbled upon the specs of the fetch standard:

https://fetch.spec.whatwg.org/#http-redirect-fetch

It states :

If one of the following is true

  • actualResponse’s status is 301 or 302 and request’s method is POST

  • actualResponse’s status is 303 and request’s method is not GET or HEAD

then:

  • Set request’s method to GET and request’s body to null.

  • For each headerName of request-body-header name, delete headerName from request’s header list.

@michael-o
Copy link
Member

@michael-o michael-o commented Apr 15, 2020

fetch is for browsers, not sure whether this applies here.

@PofMagicfingers
Copy link
Author

@PofMagicfingers PofMagicfingers commented Apr 15, 2020

I'm pretty sure following redirect in fetch is basic http implementation. I'm not sure while it would differ.

fetch is after all, implementing the http spec as any user agent would.

@jay
Copy link
Member

@jay jay commented Apr 15, 2020

In 4.3.4 it says "appropriate" 3xx response you can keep PUT, but is not more specific than that. Since PUT is meant to replace the specified resource one could argue 303 is not appropriate since a 303 server redirect is to something that represents (but isn't AFAICT) the resource, in other words see this other resource. OTOH the current behavior has been in place a long time and I don't think anybody has brought this up before.

@PofMagicfingers
Copy link
Author

@PofMagicfingers PofMagicfingers commented Apr 16, 2020

RFC2616 does say something really interesting about this :

10.3.4 303 See Other

The response to the request can be found under a different URI and
SHOULD be retrieved using a GET method on that resource.

[...]

  Note: Many pre-HTTP/1.1 user agents do not understand the 303
  status. When interoperability with such clients is a concern, the
  302 status code may be used instead, since most user agents react
  to a 302 response as described here for 303.

And few lines before, in 10.3.3 302 Found :

Note: RFC 1945 and RFC 2068 specify that the client is not allowed
to change the method on the redirected request. However, most
existing user agent implementations treat 302 as if it were a 303
response, performing a GET on the Location field-value regardless
of the original request method
. The status codes 303 and 307 have
been added for servers that wish to make unambiguously clear which
kind of reaction is expected of the client.

I'm kind of lost on which RFC is the latest we should follow, but it seems to me :

  • 303 means : do a new get request to that url
  • 307 means : repeat the request to that url

I do understand no-one has brought this up, as it might not be a common use case, although many ressources on the web claims REST APIs, or server implementations should respond with a 303 redirect after a ressource was updated with post or put.

I just found RFC7231 (dated June 2014 and seems to be be latest spec) seems to aknowledge this kind of issues.

Section 6.4.
[...]

Note: In HTTP/1.0, the status codes 301 (Moved Permanently) and
302 (Found) were defined for the first type of redirect
([RFC1945], Section 9.3). Early user agents split on whether the
method applied to the redirect target would be the same as the
original request or would be rewritten as GET. Although HTTP
originally defined the former semantics for 301 and 302 (to match
its original implementation at CERN), and defined 303 (See Other)
to match the latter semantics, prevailing practice gradually
converged on the latter semantics for 301 and 302 as well. The
first revision of HTTP/1.1 added 307 (Temporary Redirect) to
indicate the former semantics without being impacted by divergent
practice. Over 10 years later, most user agents still do method
rewriting for 301 and 302; therefore, this specification makes
that behavior conformant when the original request is POST.

If I understand this correctly (English is not my native language) :

  • They do say 301/302 redirects were defined to keep the original method
  • And 303 redirects were defined to rewrite the requests with GET method
  • But many user agent did rewrite to GET method anyway, regardless of the 301/302/303 code.
  • They added in http/1.1 a 307 redirect defined to keep the method for the redirection.

For the record, in the same document 307 redirect are defined as follow :

6.4.7. 307 Temporary Redirect

The 307 (Temporary Redirect) status code indicates that the target
resource resides temporarily under a different URI and the user agent
MUST NOT change the request method if it performs an automatic
redirection to that URI. Since the redirection can change over time,
the client ought to continue using the original effective request URI
for future requests.

@jay jay removed the not-a-bug label Apr 16, 2020
jay added a commit to jay/curl that referenced this issue Apr 16, 2020
Prior to this change if there was a 303 reply to a PUT request then
the subsequent request to respond to that redirect would also be a PUT.
It was determined that was most likely incorrect based on the language
of the RFCs. Basically 303 means "see other" resource, which implies it
is most likely not the same resource, therefore we should not try to PUT
to that different resource.

Refer to the discussion in curl#5237 for more information.

Fixes curl#5237
Closes #xxxx
@jay
Copy link
Member

@jay jay commented Apr 16, 2020

Please let me know if #5248 solves it.

@jay jay closed this in c0e139a Apr 22, 2020
@awwright
Copy link

@awwright awwright commented Apr 23, 2020

I have a few comments on this thread:

  • HTTP does not define the term "verb", maybe this is causing some confusion. I would refer to GET/POST as methods.
  • The authoritative registry of status codes is the IANA HTTP Status Code Registry; it is updated to reference the latest specifications as they get updated or replaced. If you don't know how to handle a status code, refer to this registry first.
  • Presently, all of the redirection status codes are defined in RFC 7231 (HTTP Semantics) and 308 (Permanent Redirect) is in RFC 7538
  • 303 See Other, as the name suggests, is pointing to a different resource that is talking about the request you just made. It doesn't make sense to re-issue the same request to a different resource. In contrast, 301/302/307/308 are pointing to the same resource at a different URI.
  • RFC 7231 specifically suggests a "retrieval request targeting that URI (a GET or HEAD request if using HTTP)" (emphasis mine)
  • It also says "It is primarily used to allow the output of a POST action to redirect the user agent to a selected resource ... that can be separately ... cached, independent of the original request." and you cannot cache a POST request (by default)

In summary:

  • 301 Moved Permanently — The request was not handled, and should be re-made at the new URI, and links to the old URI should be updated. The method is supposed to remain the same, but a POST may be changed to a GET for historical reasons.
  • 302 Found — The request was not handled, and should be re-made at the new URI. The method is supposed to remain the same, but a POST may be changed to a GET for historical reasons.
  • 303 See Other — The request was handled by the server, and the Location header points to a resource that is about this request. The client makes a new retrieval request (a GET or HEAD request).
  • 307 Temporary Redirect — The request was not handled, and should be re-made at the new URI. There is no option to change the method, the request must be identical except for the request-URI.
  • 308 Permanent Redirect — The request was not handled, and should be re-made at the new URI, and links to the old URI should be updated. There is no option to change the method, the request must be identical except for the request-URI.

And here's a handy table:

Status CodeRequest handled?Update bookmarks?2nd methodPOST to GET legal
301No👎Yes👍SameOK👍
302No👎No🚫SameOK👍
303YesNo🚫GET/HEADN/A
307No👎No🚫SameNo🚫
308No👎Yes👍SameNo🚫
@michael-o
Copy link
Member

@michael-o michael-o commented Apr 23, 2020

@awwright why did you mark 303 and POST to GET as "No"?

@awwright
Copy link

@awwright awwright commented Apr 23, 2020

@michael-o You're right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants
You can’t perform that action at this time.