Relative URI references not handled when exceeding `number_of_redirections`

[d2r]

When ATS is configured with proxy.config.http.redirection_enabled set to 1 and proxy.config.http.number_of_redirections set to N, ATS will follow up to N redirects seen from origin hosts.

RFC 7231 § 7.1.2 allows for the Location header field to contain a relative URI reference. This means that the value of the Location can contain a URI without a host name.

If ATS follows N redirects, its behavior for the N+1st redirect is to reply with the origin's response to the client. The problem with this is that the origin host may change in the course of following redirects 1 through N, and so the final redirect response could result in the wrong effective URI being followed by the client.

Example:

Given proxy.config.http.redirection_enabled=1 and proxy.config.http.number_of_redirections=1, consider the sequence:

1. Client -> ATS:GET foo.test/
2. ATS -> foo.test: GET foo.test/
3. ATS <- foo.test: 302 Redirect to bar.test/
4. ATS -> bar.test: GET bar.test/ (the redirect is followed by ATS)
5. ATS <- bar.test: 302 Redirect to /kau
6. Client <- ATS: 302 Redirect to /kau (the redirect is not followed ATS)

At this point the client will follow RFC 7231 § 7.1.2 regarding the value of the Location field:

the final value is computed by resolving it against the effective request URI

Therefore the Client would attempt to follow the redirect to foo.com/kau when the intended URI is bar.com/kau.

The missing host in the Location URI must be replaced with the correct value in cases when it is different from the effective client request. But the correct value of the missing host is dependent upon the architecture of which ATS is a part, and it may not be known to ATS via any existing configuration. Therefore, a plugin must be used to correct such cases.

Given that there is no general solution, it may be better to configure the ATS behavior in the case when the N+1st redirect has been returned from the origin: Currently ATS returns the redirect response to the origin, but perhaps it would be better in many architectures to respond with a 502 or otherwise reject the request if too many redirects are encountered.

[d2r]

This is still an issue.

Per @zwoop's instructions, we should remove the 8.0.0 milestone from this issue, but I do not have permission.

[d2r]

This issue is still valid.

[bryancall]

@brbzull0 Do you want to take a look at this and write a test for it?

[brbzull0]

@brbzull0 Do you want to take a look at this and write a test for it?

Sure, will do. Thanks please assign this to me.

[brbzull0]

I've put together a unit tests which does what's described in this issue.

Notes: I've used foo.test/ping & bar.test/pong instead of foo.test & bar.test for this test

curl output:

> GET http://foo.test/ping HTTP/1.1
> Host: foo.test
> User-Agent: curl/7.69.1
> Accept: */*
> Proxy-Connection: Keep-Alive
> uuid: issue2742
> 
...
< HTTP/1.1 302 Redirect
< location: /kau
< Date: Mon, 24 May 2021 15:00:54 GMT
< Age: 0
< Transfer-Encoding: chunked
< Proxy-Connection: keep-alive
< Server: ATS/10.0.0
< 
...
* Issue another request to this URL: 'http://foo.test/kau'
...
> GET http://foo.test/kau HTTP/1.1
> Host: foo.test
> User-Agent: curl/7.69.1
> Accept: */*
> Proxy-Connection: Keep-Alive
> uuid: issue2742
> 
....
< HTTP/1.1 200 OK
< host: foo.test
< Date: Mon, 24 May 2021 15:00:54 GMT
< Age: 0
< Transfer-Encoding: chunked
< Proxy-Connection: keep-alive
< Server: ATS/10.0.0
< 

I had a look at this and it seems they expect the Location to be a fully qualified url in cases like this(haven't tried that though)

Should ATS enforce the Location to some format?

I think we need to discuss how to proceed with this issue.