/redirect-to POST, added support for url/status_code in form-data

[javabrett]

Here's a further fix for /redirect-to POST support as discussed in #476. The previous change allowed POST to send url and status_code in form-data, but broke query-string support for same (a-la GET)

url (required) and status_code can now still appear in the query-string for GET or POST, but for POST/PATCH/PUT if they appear in the body form-data then the values from there are used in-favour of any in the query-string. This allows testing "traditional" POST with a form payload.

/cc #476 @eturk1

[vercel]

Since this Pull Request originated from a forked repository, Now cannot deploy it as there are potential security risks.

If you are a collaborator on this repository, consider making this Pull Request from a branch on the same repository instead of a fork.

[eturk1]

Shouldn't /redirect-to only use the method in the http header GET or POST and look only in the correct place?
Thus if someone uses only a query string on a POST there would be an error about missing form/post data?

Here's a further fix for /redirect-to POST support as discussed in #476. The previous change allowed POST to send url and status_code in form-data, but broke query-string support for same (a-la GET)
url (required) and status_code can now still appear in the query-string for GET or POST, but for POST/PATCH/PUT if they appear in the body form-data then the values from there are used in-favour of any in the query-string. This allows testing "traditional" POST with a form payload.
/cc #476 @eturk1

[javabrett]

Shouldn't /redirect-to only use the method in the http header GET or POST and look only in the correct place?

Please consider supplying an RFC or spec-reference for what might be incorrect here. I don't believe there is anything wrong with supplying query-string parameters to a HTTP POST. I also think how they are interpreted and/or combined with any form-data is application-specific.

Sure, form-data appears in the POST body, but nothing precludes the form action from containing query-string parameters.

Thus if someone uses only a query string on a POST there would be an error about missing form/post data?

Again, I don't think there's anything necessarily erroneous about a) query-strings in form actions or b) POST with empty body. If you feel this is the case, please supply references.

--

The debate here I think is about how flexible redirect-to should be and how mixed-parameters should be handled. I feel pretty strongly that it should accept the parameters via formData, hence this PR. I feel much less comfortable about removing the ability to send the parameters in the query-string, even if it is POST, that seems a valid case to me if less-common. What could be debated is whether the service should throw an error if you try to mix them, but I can't see why it should other than that the caller needs to know which takes precedence (I propose POST form-data).

[eturk1]

HTTP/1.1 spec: https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html
The Request-Line specifies method (POST/GET) with either 1-line RequestLine for GET & for POST to look in body for parms. Nothing in the spec about POST method expecting/reading data in the URL.

In example headers, it's poor form to include query string on a POST method because it will be ignored at best. https://www.w3schools.com/tags/ref_httpmethods.asp

I'd say that using URL query string in a POST is unneeded code that's confusing to maintain and supports improper support of a POST with URL. i.e. switch on request method for where to look for parms.

[javabrett]

According to https://tools.ietf.org/html/rfc7230 :

HTTP-message = start-line
*( header-field CRLF )
CRLF
[ message-body ]

start-line = request-line / status-line

request-line = method SP request-target SP HTTP-version CRLF

request-target = origin-form
/ absolute-form
/ authority-form
/ asterisk-form

origin-form = absolute-path [ "?" query ]

And I can't see anything in https://tools.ietf.org/html/rfc7231 that forbids a query for POST.

So I think it comes down to how the application wants to interpret any query in the resource URI, right? Perhaps you are saying that it is improper or unconventional to interpret this way. But is it no-good according to specs - I'm not seeing it, please provide references.

[eturk1]

Brett, specs will not be cluttered with every possibility that should NOT be done. Agreed?

Note the GET spec in first sentence states:
"The GET method means retrieve whatever information (in the form of an entity) is identified by the Request-URI."
https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.3

There is no mention in GET of retrieving or not retrieving anywhere else, therefore anywhere but URI line is ignored. Thus the same for POST spec, we read from the body, as spec'd, and ignore anywhere else.

[javabrett]

@eturk1 better to refer to the newer RFCs which obsolete RFC2616, especially RFC7230 and RFC7231.

Brett, specs will not be cluttered with every possibility that should NOT be done. Agreed?

Sorry, I can't agree in this case. The specs provide a very clear BNF of what types of requests are allowed for each verb, and that is the "word" so-to-speak. If it's valid according to the BNF of the protocol, it's valid. There's no wording to suggest anything about this being disallowed or even not-recommended. Claiming it should not be done according to some supposed convention is an overreach.

And I think you will find RFC3875 CGI mandates that they be allowed.

Golang found that they weren't processing the query-string for POST in their API, and fixed a bug - see their bug 985.

There's significant other commentary on the web about query-string being allowed for POST. Can you reference some reliable commentary that they are always invalid or ignorable?

[eturk1]

I suggest we get a third person familiar with http spec to comment. :) I just don't anywhere reading the query on a POST method in the request.

And I think you will find RFC3875 CGI mandates that they be allowed.
"CGI" is server-side only, not about client/server request-response:
https://tools.ietf.org/html/rfc3875#section-1.1

Let's look at an objective source. Are you saying the lede here is wrong and needs to be corrected?
https://en.wikipedia.org/wiki/POST_(HTTP)
Would the correction be: "POST can be in body OR query... In contrast, GET is only in query."?

Would now binary data, passwords, etc. be passed on query to POST request or only text?
If both body & query data were passed, which would be ignored?
This is why logically the absence of query from spec is because it's ignored by servers. There's just too many what-ifs that would have to be in spec to support query also.

[javabrett]

I'd like to suggest that you stop focusing on trying to show that POST requests cannot have a query (they can according to the specs), or that it is always invalid (it isn't), and rather focus your attention on expressing that you don't like or want this application (httpbin) to accept them, specifically for the redirect-to interface. Even better, just focus the battle on getting this PR merged in the first place, so that at least you can include the parameters in a POST body, per your original bug.

Also drawing attention to the fact that the reason the change failed is that it failed a unit test when it stopped listening to the query-string ... making your suggestion a non-backward-compatible change, one that developers might already rely on in their test-suites.

I suggest we get a third person familiar with http spec to comment.

Sure, bring who you like.

I just don't anywhere reading the query on a POST method in the request.

It's in the BNF I quoted earlier. Not about reading it, which is application-specific, but about it being allowed and valid.

Let's look at an objective source.

With all respect, it is descending into silly when a) quote the RFCs, b) don't like what they say, c) define Wikipedia as the objective source (presumably of truth).

This is why logically the absence of query from spec is because it's ignored by servers.

It's not absent, it is there, please read the (newest!) RFCs again.

I'm not sure what you mean by it's ignored by servers. Do you mean HTTP servers, CGI Gateways, or applications (all of them) which run there?

You should know that in the CGI spec and implementations, QUERY_STRING is set for a POST. Try it out if you like. Of course the application (in our case httpbin) is free to use this or discard it as it prefers when working out the POST resource (or script). You might be claiming that all CGI implementations have a bug if they set QUERY_STRING for a POST ...?

I really do think that you have mixed-up the query string and the post body here, because of the way GET forms work is to use the query for the form values. You can't use that argument to ban query from POST, not from a specifications perspective anyway.

RFC3875:

4.1.7. QUERY_STRING
...
The QUERY_STRING value provides the query-string part of the
Script-URI. (See section 3.3).

3.3. The Script-URI
...
From the meta-variables thus generated, a URI, the 'Script-URI', can
be constructed. This MUST have the property that if the client had
accessed this URI instead, then the script would have been executed
with the same values for the SCRIPT_NAME, PATH_INFO and QUERY_STRING
meta-variables.

[eturk1]

Agreed! What's important is a POST method request with body params be processed correctly.
If server wants to also process a POST by reading query that is secondary.

Will just assume #476 will be addressed here in #482

thanks!

[javabrett]

@eturk1 do you have access to reopen #476?

[eturk1]

don't think I can change an issue status to open