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

HTTP HEAD request returns 403 #13079

Open
1 task
fhennig opened this issue Jan 27, 2025 · 5 comments · May be fixed by #13101
Open
1 task

HTTP HEAD request returns 403 #13079

fhennig opened this issue Jan 27, 2025 · 5 comments · May be fixed by #13101
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@fhennig
Copy link

fhennig commented Jan 27, 2025

Astro Info

Astro                    v5.1.10
Node                     v23.4.0
System                   macOS (arm64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/node
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

When using SSR and the standalone server in node, a HEAD request will return a 403 response.

What's the expected result?

A 200 response.

Link to Minimal Reproducible Example

https://github.com/fhennig/astro-head-403

Participation

  • I am willing to submit a pull request for this issue.

In addition to the linked example, you can reproduce with the following steps:

  • Create an astro app: npm create astro@latest
  • Install the node plugin: npm i @astrojs/node
  • Configure SSR:
import { defineConfig } from 'astro/config';
import node from '@astrojs/node'; // or another adapter

export default defineConfig({
  output: 'server', // Enable SSR
  adapter: node({ mode: 'standalone' }),
});
  • Build: npm run build
  • serve: node ./dist/server/entry.mjs
  • do a head reaquest: curl --head http://localhost:4321

We noticed this because some uptime checkers do HEAD requests and showed our site as being down.

@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jan 27, 2025
@corneliusroemer
Copy link

corneliusroemer commented Jan 27, 2025

We first encountered this with Astro v5 on 2 projects using Astro.

Note: The bug does not surface when you do a npm run dev - one needs to build and serve the production version.

When bisecting in the loculus-project/loculus codebase, astro v4.16.17 was still working fine and v5.0.2 was bad.

Specific versions that made it bad:

@astrojs/node: 8.3.4 -> 9.0.0
astro: 4.16.17 -> 5.0.2
@astrojs/react: 3.6.3 -> 4.0.0

@corneliusroemer
Copy link

corneliusroemer commented Jan 27, 2025

I can reproduce within the astro code base using example/ssr and the following commands:

pnpm i
pnpm build
pnpm --filter @example/ssr i
pnpm --filter @example/ssr build
pnpm --filter @example/ssr preview &
curl -I http://localhost:4321

Bisecting yields the following as the first bad commit: #12588

@joshmkennedy
Copy link
Contributor

I believe this is the line of code that is causing the issue

const sameOrigin =
			(request.method === 'POST' ||
				request.method === 'PUT' ||
				request.method === 'PATCH' ||
				request.method === 'DELETE') &&
			request.headers.get('origin') === url.origin;

request.method === 'HEAD' should be added to this list

@joshmkennedy
Copy link
Contributor

Actually, I was wrong.

This fixes it, which is the line above what I pasted mentioned.

	if (request.method === 'GET' || request.method === "HEAD") {
	return next();
	}

I submitted a pull request here #13100 with the fix

@corneliusroemer
Copy link

Good spot @joshmkennedy! The bug was introduced in 315c5f3#diff-aeb15293e9b48a0df451f282702d3145060faecc40e2360faf3d9b059c862f1f where forbidden methods (POST, PATCH, DELETE,...) were wrongly inverted to just GET (omitting HEAD)

@ematipico ematipico added the - P3: minor bug An edge case that only affects very specific usage (priority) label Feb 4, 2025
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Feb 4, 2025
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.

4 participants