Allow illegal redirect responses

[triblondon]

An HTTP response that has a redirection status but no Location header is invalid per the Fetch spec, but it's still a valid HTTP response. It seems reasonable that a developer may want to receive and parse the response as normal, and there should be an escape valve from the illegal redirect error.

This strikes me as a similar case to other browser-security-model rules of fetch, such as suppressing certain headers, rules which are required in the browser but make fetch-h2 less useful as a generic HTTP client server-side. Therefore my tentative suggestion is to disable illegal redirect errors if the existing allowForbiddenHeaders flag is true.

I also considered suggesting an additional flag, but that seems like overkill to me. However, it does leave the flag slightly awkwardly named, and I guess enforceValidation might be a more generic way to refer to it if you agree to bundle these behaviours into one flag.

[coveralls]

Coverage increased (+0.09%) to 83.193% when pulling 90dd7e3 on triblondon:patch-2 into 89236f4 on grantila:master.

[coveralls]

Coverage increased (+0.09%) to 83.193% when pulling 90dd7e3 on triblondon:patch-2 into 89236f4 on grantila:master.

[grantila]

Hmm, what can you do with such a response? Is it at all useful? Surely it's proper HTTP (if the headers and body is well-formed), but semantically it's similar to a broken link.
"Please go to the url undefined".

The forbidden headers are mostly for security which "only" applies to browsers, that's true, but this is more of a way to handle actual semantic bugs, right?

[triblondon]

My use case is an HTTP client, so I want to be able to show the response that was received and report that the location header was missing. But right now we are losing all the data and just getting an exception.

[colinbendell]

+1 for @triblondon's use case. Like him, we are using fetch-h2 in one part as a triage tool to detect misbehaving servers. Escape valves like this are useful for the savvy client (though clearly not intended for the most common use cases)

[triblondon]

Hi @grantila when you have time could you give us some indication of whether you'd be willing to accept this or some other solution to this problem? Ie. let's start with: do you accept this use case as something fetch-h2 can support? If not, we can close the issue.

But if so, then I feel like the proposed PR is one possible option, and happy to discuss others.

[grantila]

Sorry for the delay @triblondon and @colinbendell, I agree with you.

I'm starting to think that fetch-h2 should not be Fetch API compliant by default, in that it should allow "illegal" headers, and it should allow this kind of "broken" response etc. To act more like browser fetch, the option {allowForbiddenHeaders: true|false} should be converted into a {compliance: 'strict'|'loose'} where loose is the default (not sure about the naming though).

This will make fetch-h2 more useful as an arbitrary http client by default.

It would however be a breaking change, but probably worth it.

[triblondon]

Agreed. If this would be the only breaking change you have on the horizon, you could leave it in compliance:strict mode by default. I'd be happy to throw in the extra param to turn it off. Then you could flip to loose-by-default on the next major release?

[triblondon]

@grantila should I close this? Wether or not the library provides some solution to the problem I was trying to address, it seems like it won't be this solution, right?