ring-clojure / ring-headers

Ring middleware for common response headers
26 stars 12 forks source link

Absolute Redirects Middleware RFC compliance #2

Closed kbaribeau closed 8 years ago

kbaribeau commented 9 years ago

Hi,

The absolute redirects middleware claims that the "HTTP RFC" does not allow relative URIs in the Location header.

This was true for RFC 2616, but is no longer true in RFC 7231, which obsoletes RFC 2616

http://tools.ietf.org/html/rfc7231#section-7.1.2

http://tools.ietf.org/html/rfc2616#page-135

I think the prudent solution is:

  1. Update the docstrings for this middleware
  2. remove absolute-redirects from the default set of middlewares in ring-defaults

(1) seems like a no brainer, I'll attach a PR. (2) might be debatable.

Thoughts?

weavejester commented 9 years ago

What's the purpose of (2)? Absolute URLs are valid for both RFCs, so there shouldn't be any issue with using this middleware, even for clients that support relative URLs.

kbaribeau commented 9 years ago

I ran into this middleware because my https urls were getting changed to http urls. There's no good solution for this middleware in my case because our SSL terminates at a load balancer. Ring is not aware of SSL in my case.

Maybe my case is unusual, I'd be willing to accept that.

The fact the relative URLs are required in RFC 7231 indicates to me that clients that do not support relative URLs are probably rare.

If there exist popular clients (IE 6 maybe?) that don't support relative URLs than that's a pretty good case for keeping this middleware in the set of defaults.

If there aren't any problematic clients out there, why mess with URLs by default?

kbaribeau commented 9 years ago

By the way, (2) just seemed like a reasonable idea to me. Feel free to just close this issue if it doesn't seem that way to you, although I'm happy to discuss.

Thanks for listening!

weavejester commented 9 years ago

If you're running a server behind a proxy, it's recommended that you set the :proxy option to true in Ring-Defaults. This corrects the :scheme and :remote-addr keys.

All major browsers have supported relative redirects for some time, so I'm not particularly worried about that. What concerns me is how many HTTP client libraries lack support for relative redirects, and if you indicate to Ring-Defaults that your server is behind a proxy, there isn't any downside I can see to making redirects absolute by default.

kbaribeau commented 9 years ago

Alright, thanks!