search

dojo / core

:rocket: Dojo 2 - language helpers and utilities.
http://dojo.io
Other
213 stars 62 forks source link

Making redirects follow (more closely anyway) the RFC, fixes #207 #212

Closed rorticus closed 7 years ago

rorticus commented 7 years ago

This should more closely follow an RFC.. I used this to find the requirements,

https://www.w3.org/Protocols/rfc2616/rfc2616.txt

In the places where the RFC is unclear, or where browsers (Chrome/Firefox) deliberately ignore the RFC, I tried to match them and call out in the code when that happens.

The RFC states that clients should "figure it out" when it comes to infinite redirect loops. I wasn't sure if we wanted to get into that or not, so I'm just setting a (configurable) limit on the number of redirects we'll allow before we bail.

I also decided to include a keepOriginalMethod configuration to ignore the best practices and instead redirect how you think it should redirect, i.e., POST requests redirect to POST requests. I worked on an API once that had a very mean load balancer in front of it that would forward all my POST requests to another server with a 301 redirect. If the client I was working with didn't have a "keep original method" option, I would not have been able to interface with the API.

Anyways, let me know what you think!

novemberborn commented 7 years ago

In the places where the RFC is unclear, or where browsers (Chrome/Firefox) deliberately ignore the RFC, I tried to match them and call out in the code when that happens.

RFC 2616 has been obsoleted in favor of https://tools.ietf.org/html/rfc7230. Not sure if anything is more clear in the newer spec though.

rorticus commented 7 years ago

RFC 2616 has been obsoleted

Thanks, I'll check it out and see if anything is different. I'm not sure whats up with the CI stuff, I'm pretty sure all the tests pass locally, but I'll take a look at that too.

kitsonk commented 7 years ago

I wonder if there was a regression/change in NodeJS v6.8.0 (which is what Travis is using).

rorticus commented 7 years ago

RFC 2616 has been obsoleted

OK so RFC 7230 is remarkably silent on redirects (redirects are only mentioned in passing, when referring to how they affect the actual http protocol).

rorticus commented 7 years ago

I wonder if there was a regression/change in NodeJS v6.8.0 (which is what Travis is using).

I don't know.. I just ran the tests locally with node 6.8.1 and they all passed.

kitsonk commented 7 years ago

I restarted the build on Travis to see if was just in Travis mind... ;-)

codecov-io commented 7 years ago

Current coverage is 97.46% (diff: 100%)

No coverage report found for master at c06ee1d.

Powered by Codecov. Last update c06ee1d...d1df077