grantila / fetch-h2

HTTP/1+2 Fetch API client for Node.js
MIT License
336 stars 16 forks source link

Allow forbidden response headers? #30

Closed triblondon closed 5 years ago

triblondon commented 5 years ago

I notice that when I iterate response.headers, I don't see Set-Cookie headers. There's an allowForbiddenHeaders option to override guards for requests, but there seems to be no way to override the guards for responses.

My use case is an HTTP debugging tool, so it's actually important that the client reports the headers that were actually received. It does seem unnecessary to me to enforce these guards: I realise they are part of the fetch spec, but fetch was designed for browsers, and while the API is really nice and it makes sense to use it on servers as well, the security considerations are a bit different. Having the tight spec conformance is good in and of itself, so really we just need a way to opt out of it (and I'd personally argue that would be a helpful default state!)

Happy to help with this if you agree and can point me in the right direction.

grantila commented 5 years ago

This is now implemented, either with a allowForbiddenHeaders when you create the Response manually, or adopted through the fetch call.

For push-promise requests, it is hard-coded to true. If you need this customizable (though the context preferably), just re-open or open a new issue, thanks.

grantila commented 5 years ago

:tada: This issue has been resolved in version 2.0.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

triblondon commented 5 years ago

I'm afraid I don't think this is working yet. I imagine you need to delete these lines:

https://github.com/grantila/fetch-h2/blob/master/lib/fetch-http2.ts#L252-L253

And maybe these?

https://github.com/grantila/fetch-h2/blob/master/lib/fetch-http1.ts#L166-L167

I'm not sure what the intent of these lines is. Possible leftover code from a prior, less sophisticated solution to guarding headers?

grantila commented 5 years ago

Oh yeah, well the intent is to mimic browsers and the Fetch API in which these headers aren't available, but transparently forwarded on future requests to the same origin. I added the CookieJar support so that you use that instead of reading the cookies manually through the headers..

grantila commented 5 years ago

This is now fixed. Tell me if you find more issues. You're using fetch-h2 in non-"Fetch API" ways, and that's a feasible thing to do, or at least should be, since we don't have the same limitations in Node.js as we do in a browser.

grantila commented 5 years ago

:tada: This issue has been resolved in version 2.0.6 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

triblondon commented 5 years ago

Confirmed working! Many thanks.