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

SSR Nodejs (middleware): HTTP2 requests have an undefined hostname #7135

Closed
1 task done
bmenant opened this issue May 19, 2023 · 1 comment · Fixed by #7215
Closed
1 task done

SSR Nodejs (middleware): HTTP2 requests have an undefined hostname #7135

bmenant opened this issue May 19, 2023 · 1 comment · Fixed by #7215
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@bmenant
Copy link
Contributor

bmenant commented May 19, 2023

What version of astro are you using?

2.5.0

Are you using an SSR adapter? If so, which one?

Node

What package manager are you using?

npm

What operating system are you using?

Linux

What browser are you using?

Firefox

Describe the Bug

Run as a middleware using @fastify/middie, @astro/image integration fails to transform images (no matters which transformers is used) from HTTP/2 requests.

As a matter of fact, requests passed down to the Astro handler are properly routed to image endpoints, but local images can’t be loaded since hostname part of URLs are undefined (the loadRemoteImage() function receives https://undefined/... source URLs).

The issue comes from the createRequestFromNodeRequest() function:
https://github.com/withastro/astro/blob/main/packages/astro/src/core/app/node.ts#L17

HTTP2 Requests Headers from Fastify do not have the host property, but a :authority pseudo header (see also https://nodejs.org/docs/latest-v18.x/api/http2.html#note-on-authority-and-host).

How about using a fallback req.headers.host || req.headers[':authority'] instead?

I’ll push a merge request later.

Link to Minimal Reproducible Example

https://will.come.later/when/i/find/some/time

Participation

  • I am willing to submit a pull request for this issue.
@bmenant bmenant changed the title SSR Nodejs (middlware): HTTP2 requests ends have an undefined hostname SSR Nodejs (middleware): HTTP2 requests have an undefined hostname May 20, 2023
@bluwy
Copy link
Member

bluwy commented May 25, 2023

Thanks for digging into this. The proposed change makes sense, feel free to send a PR!

@bluwy bluwy added the - P3: minor bug An edge case that only affects very specific usage (priority) label May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants