[BUG] response:redirect-to#1 adds scheme and host to URL

[line-o]

Describe the bug

Calling response:redirect-to(xs:anyURI("/exist/apps/monex/index.html")) to issue a redirect with status code 302 will add the scheme and host to the URL.

➜ curl http://localhost:8080/exist/apps/test/redirect-to.xq -I
HTTP/1.1 302 Found
Date: Sun, 20 Feb 2022 21:52:55 GMT
X-XQuery-Cached: true
Location: http://localhost:8080/exist/apps/monex/index.html
Content-Length: 0
Server: Jetty(9.4.44.v20210927)

Expected behavior

The temporary redirect location header to be an absolute or relative URL (see also https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location).

➜ curl http://localhost:8080/exist/apps/test/redirect-to.xq -I
HTTP/1.1 302 Found
Date: Sun, 20 Feb 2022 21:52:55 GMT
X-XQuery-Cached: true
Location: /exist/apps/monex/index.html
Content-Length: 0
Server: Jetty(9.4.44.v20210927)

To Reproduce

xquery version "3.1";

module namespace t="http://exist-db.org/xquery/test";

declare namespace test="http://exist-db.org/xquery/xqsuite";
declare namespace hc="http://expath.org/ns/http-client";

declare
    %test:setUp
function t:setup() {
    let $testCol := xmldb:create-collection("/db", "apps")
    let $testCol := xmldb:create-collection("/db/apps", "test")
    return
        xmldb:store("/db/apps/test", "redirect-to.xq",
            "response:redirect-to(xs:anyURI('/exist/apps/monex/index.html'))")
};

declare
    %test:tearDown
function t:tearDown() {
    xmldb:remove("/db/apps/test")
};

declare
    %test:assertTrue
function t:test() {
    let $request :=
        <hc:request method="get" follow-redirect="false"
            href="http://localhost:8080/exist/apps/test/redirect-to.xq" />

    let $head := hc:send-request($request)[1]

    return
        xs:integer($head/@status) eq 302 and 
        $head//hc:header[@name = 'location']/@value/string() eq '/exist/apps/monex/index.html'
};

Context (please always complete the following information):

• OS: macOS 10.15.7
• eXist-db version: 6.0.0
• Java Version Java8u322

Additional context

• How is eXist-db installed? built from source eXist-6.0.0
• Any custom changes in e.g. conf.xml? none

[line-o]

related eXist-db/public-repo#76

[joewiz]

Using eXist 6.1.0-SNAPSHOT 94857f2 20220211055142, I can confirm that the xqsuite demonstrates the issue.

[dizzzz]

I expect the actual code to be present in a different repo

[joewiz]

@dizzzz Could you clarify?

[line-o]

@joewiz We discussed this issue in the community call and I asked where to find the implementation of the redirect-to function. That is what @dizzzz is referring to.

[joewiz]

@line-o Ah, I see! Were you able to find it? If not, it appears to be the one result here: https://github.com/eXist-db/exist/search?q=redirect-to.

[line-o]

@joewiz No that just calls the redirectTo method of a class that extends or implements the ResponseWrapper. But I haven't found the actual implementation of redirectTo that is used.

[line-o]

According to the Javadoc of HTTPServletResponse.sendRedirect the behaviour we are observing should not be happening. That looks to me as if the ResponseWrapper implementation that is used here would interfere.

[line-o]

So digging deeper it appears to be a jetty specific behaviour. The host header is used to build absolute URLs including scheme and host.
BUT, fortunately there is now an option to put an end to this (see jetty/jetty.project#5029).

[line-o]

I can confirm that adding

<Set name="relativeRedirectAllowed">
    <Property name="jetty.httpConfig.relativeRedirectAllowed" default="true"/>
</Set>

to jetty.xml (for me it was at Line 78) does indeed change the behaviour so that the above tests will pass.
Should we make this a default to false and make it configurable?

[adamretter]

Thanks for confirming, as I mentioned on the call I was almost certain that this was not an eXist-db issue with redirect-to. Glad to have that confirmed!

Reading the Jetty issue that you linked, it states that:

relative redirects are now allowed by RFC 7230

and that the Jetty project have recently added an option to support this newer behavioural option. I don't think we should change the default Jetty config in eXist-db at this time, as this has:

1. been the standard behaviour in eXist-db for ages,
2. and, the relative redirect in RFC 7230 is a relatively new concept that itself may not be supported or expected by all existing HTTP clients.

I think the fact that we already expose Jetty options that allow you to solve your problem is enough for the moment.
Perhaps some further documentation in exist-docs would be called for if you think this will effect other users?
As I read it in the Jetty issue though, for not just Jetty but other more widely used web-servers (i.e. Apache HTTPD), then those and Jetty (and eXist-db by extension) use the current behaviour that we have.

@line-o Out of interest... What is the HTTP Client that you are using where you are seeing some sort of error due to this? It would seem sensible for you to issue a bug-report to them, as they should very likely support what has been the "de-facto" approach in at least Apache HTTP and Jetty for a long time already.

[adamretter]

Someone seems to have deleted my earlier comments from directly after the community-call about this being part of the Java Servlet Spec, and therefore implemented by Jetty :-(

[line-o]

I was wondering where the earlier comment went. But de-facto, just by looking at MDN there is zero need to add scheme and URL to the location header.

[line-o]

"My" HTTP-client being all major browsers.

[adamretter]

just by looking at MDN there is zero need to add scheme and URL to the location header.

That depends on which RFCs you are supporting. The MDN page you referenced does not explain which version of the RFCs they are discussing. In addition that page does say that the Location header can be "A relative (to the request URL) or absolute URL.", which seems to say that MDN believe that either version is acceptable.

My" HTTP-client being all major browsers

As far as I can see, eXist-db's (i.e. Jetty's) behaviour works with "all major browsers" - I quickly tested fairly recent Chrome, Firefox, Safari and IE browsers.

@line-o Can you tell us, what is the actual error that you are experiencing?

For reference, with Google Chrome as the client, the TCP dump trace from Wireshark looks like:

GET /exist/rest/db/redirect-to.xq HTTP/1.1
Host: localhost:8080
Connection: keep-alive
sec-ch-ua: " Not A;Brand";v="99", "Chromium";v="99", "Google Chrome";v="99"
sec-ch-ua-mobile: ?0
sec-ch-ua-platform: "Linux"
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/99.0.4844.82 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
Sec-Fetch-Site: none
Sec-Fetch-Mode: navigate
Sec-Fetch-User: ?1
Sec-Fetch-Dest: document
Accept-Encoding: gzip, deflate, br
Accept-Language: en-GB,en-US;q=0.9,en;q=0.8,de;q=0.7
Cookie: org.exist.login=deleted; JSESSIONID=node0v8cj5rc120uu1l17n3gnn5hrc0.node0; _ga=GA1.1.216916786.1642352830; JSESSIONID=node0vs7c03z1ixyz13gjad8o7fg4l4.node0

HTTP/1.1 302 Found
Date: Tue, 22 Mar 2022 15:37:43 GMT
X-XQuery-Cached: false
Location: http://localhost:8080/exist/apps/monex/index.html
Content-Length: 0
Server: Jetty(9.4.45.v20220203)

GET /exist/apps/monex/index.html HTTP/1.1
Host: localhost:8080
Connection: keep-alive
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/99.0.4844.82 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
Sec-Fetch-Site: none
Sec-Fetch-Mode: navigate
Sec-Fetch-User: ?1
Sec-Fetch-Dest: document
sec-ch-ua: " Not A;Brand";v="99", "Chromium";v="99", "Google Chrome";v="99"
sec-ch-ua-mobile: ?0
sec-ch-ua-platform: "Linux"
Accept-Encoding: gzip, deflate, br
Accept-Language: en-GB,en-US;q=0.9,en;q=0.8,de;q=0.7
Cookie: org.exist.login=deleted; JSESSIONID=node0v8cj5rc120uu1l17n3gnn5hrc0.node0; _ga=GA1.1.216916786.1642352830; JSESSIONID=node0vs7c03z1ixyz13gjad8o7fg4l4.node0

HTTP/1.1 200 OK
Date: Tue, 22 Mar 2022 15:37:43 GMT
Last-Modified: Tue, 22 Mar 2022 15:34:23 GMT
Created: Tue, 22 Mar 2022 15:34:23 GMT
Cache-Control: no-cache
X-XQuery-Cached: false
Content-Type: text/html;charset=utf-8
Vary: Accept-Encoding, User-Agent
Content-Encoding: gzip
Transfer-Encoding: chunked
Server: Jetty(9.4.45.v20220203)

...

[joewiz]

@adamretter Are these the comments you mentioned as having been deleted? #4264 (comment).

[adamretter]

@joewiz Ahaha - so not deleted - just added to the wrong issue! Thanks :-)

[line-o]

There are three forms of absolute URLs

1. starts with a slash followed by the full path to a resource
2. starts with a scheme followed by the host and the full path to a resource
3. starts with two slashes followed by the host and the full path to the resource

When redirecting to a resource there is no need to include the host, if the resource is known to be on the same one as the initial request went to. In the case of the linked issue this means that either 1 or 3 would have been fine.

Jetty, however uses the host header of the initial request and adds that to the location header of the response together with the scheme it was called with. This raises a series of issues:

• might lead to open redirects as the host header might be tampered with
• does not work when the proxy terminates ssl and redirects internally with http
• does not work if the proxy changes the host header

All of the above is fixed by "allowing" jetty to have absolute URLs in the location header that start with a slash while redirecting other hosts using fully-qualified URLs (2,3) will still continue to work.

[adamretter]

Jetty, however uses the host header of the initial request and adds that to the location header of the response together with the scheme it was called with. This raises a series of issues:

Are these actually real-world issues? - As we know that Apache HTTPD Server does the same as Jetty, and Apache HTTPD is likely the most widely deployed web-server in the world!

does not work when the proxy terminates ssl and redirects internally with http
does not work if the proxy changes the host header

I actually have that working on plenty of servers for many years now where my reverse-proxy is nginx.

I would still like to know what the actual error is that you have seen...