Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting pseudo-headers for H2 request #2307

Closed
kyrylodolynskyi opened this issue Oct 4, 2023 · 6 comments
Closed

Setting pseudo-headers for H2 request #2307

kyrylodolynskyi opened this issue Oct 4, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@kyrylodolynskyi
Copy link
Contributor

kyrylodolynskyi commented Oct 4, 2023

Bug Description

I want to use undici with the allowH2 flag to overcome CloudFlare anti-DDoS behavior, this requires :method, :authority, :path, and :scheme pseudo-headers to be sent, but undici doesn't add them on its own (as browser does) and doesn't allow to set them by myself, since the header key validation from http1.1 is applied.

Reproducible By

import {fetch, Client} from 'undici'

const client = new Client(`<http2 server>`, {
  allowH2: true,
})

await fetch('<http2 resource>', {
  dispatcher: client,
  headers: {
    ':authority': '<authority>',
    ':method': 'GET',
    ':path': '<path>',
    ':scheme': 'https'
  }
})

Expected Behavior

The pseudo-headers are added by the fetch itself, or it is possible to pass them manually.

Environment

macOS Ventura 13.4.1, Node v20.5.0

@kyrylodolynskyi kyrylodolynskyi added the bug Something isn't working label Oct 4, 2023
@metcoder95
Copy link
Member

We already attach the :authority, :method, and :path headers; but its true the :scheme is not added. It should be a matter of add it here:

headers[HTTP2_HEADER_AUTHORITY] = host || client[kHost]

As we do not support h2c (its non-encrypted version), it is safe to hardcode it as https.

Would you like to send a PR? 🙂

@kyrylodolynskyi
Copy link
Contributor Author

@metcoder95 Oh, thank you for pointing me at this part of the code (I was looking at core/request.js 🤦🏼). I played with the code a little bit and found out that even adding :scheme doesn't solve my problem, but what solves is changing the order of the headers from :authority - :path - :method, to :authority - :method - :path, or :method - :authority - :path. Seems like at least for CloudFlare it is crucial for :method header to appear before :path.
I would like to open a PR to fix it, but I am not aware of any specification that documents that, so I am not sure if you would be open to such a change. WDYT?

@metcoder95
Copy link
Member

Feel free to open a PR for changing the order 👍

By spec, the order of the headers should not be a problem; as is not specified. But it can be obvious that some prefer a given order.
Maybe if we find more issues like this one, we'll need to allow its customization.

@amitzur
Copy link

amitzur commented Oct 4, 2023

In this case this seems like misbehavior of CloudFlare. I'm in favor of a PR that changes the order, but in parallel is there a way to influence CloudFlare and bring this to their attention?

@metcoder95
Copy link
Member

Not really sure, maybe you can open a thread on its community forum and seek of more consensus and maybe grasp is attention: https://community.cloudflare.com/

@metcoder95
Copy link
Member

Closed by #2308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants