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

https option not working in svelte-kit next.236 #3479

Closed
joshnuss opened this issue Jan 21, 2022 · 13 comments · Fixed by #3553
Closed

https option not working in svelte-kit next.236 #3479

joshnuss opened this issue Jan 21, 2022 · 13 comments · Fixed by #3553
Labels
bug Something isn't working
Milestone

Comments

@joshnuss
Copy link

joshnuss commented Jan 21, 2022

Describe the bug

Running pnpm run dev -- --https should serve the project at https://localhost:3000.

Reproduction

  1. Create a blank svelte-kit project: pnpm init svelte@next https-bug && cd https-bug && pnpm install
  2. Start the project with https enabled: pnpm run dev -- --https
  3. Open the site in the browser https://localhost:3000
  4. Browser returns a 500 error (error text below)

Logs

TypeError [ERR_INVALID_HTTP_TOKEN]: Header name must be a valid HTTP token [":method"]
    at file:///home/josh/Playground/svelte-kit-with-https/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.236_svelte@3.46.2/node_modules/@sveltejs/kit/dist/install-fetch.js:5207:6
    at Array.map (<anonymous>)
    at new Headers (file:///home/josh/Playground/svelte-kit-with-https/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.236_svelte@3.46.2/node_modules/@sveltejs/kit/dist/install-fetch.js:5206:12)
    at new Request (file:///home/josh/Playground/svelte-kit-with-https/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.236_svelte@3.46.2/node_modules/@sveltejs/kit/dist/install-fetch.js:5934:19)
    at file:///home/josh/Playground/svelte-kit-with-https/node_modules/.pnpm/@sveltejs+kit@1.0.0-next.236_svelte@3.46.2/node_modules/@sveltejs/kit/dist/chunks/index.js:1984:8
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

System Info

System:
    OS: Linux 5.13 Linux Mint 20.3 (Una)
    CPU: (8) x64 Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
    Memory: 657.21 MB / 15.52 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.4.0 - ~/.asdf/installs/nodejs/16.4.0/bin/node
    Yarn: 1.22.10 - ~/.asdf/installs/nodejs/16.4.0/.npm/bin/yarn
    npm: 8.3.0 - ~/.asdf/plugins/nodejs/shims/npm
  Browsers:
    Chrome: 97.0.4692.99
    Firefox: 96.0
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.14 
    @sveltejs/kit: next => 1.0.0-next.236 
    svelte: ^3.44.0 => 3.46.2

Severity

blocking all usage of SvelteKit for apps that require ssl.

Additional Information

No response

@rchrdnsh
Copy link

rchrdnsh commented Jan 21, 2022

I'm being hit with this error as well trying to use vite-plugin-mkcert with https enabled in the config file after updating to 234+

@joshnuss
Copy link
Author

I think it has to do with HTTP2, since HTTP2 headers can have colons : in them, ie :method:, :path:, but the regex used for parsing headers doesn't allow them.

@nstuyvesant
Copy link
Contributor

Same error with SvelteKit 1.0.0-next.239 and newer node version...

System:
OS: macOS 12.1
CPU: (10) arm64 Apple M1 Pro
Memory: 1.54 GB / 32.00 GB
Shell: 5.8 - /bin/zsh
Binaries:
Node: 16.13.2 - /opt/homebrew/bin/node
Yarn: 1.22.17 - /opt/homebrew/bin/yarn
npm: 8.3.2 - /opt/homebrew/bin/npm
Browsers:
Chrome: 97.0.4692.99
Safari: 15.2
npmPackages:
@sveltejs/adapter-auto: next => 1.0.0-next.14
@sveltejs/kit: next => 1.0.0-next.239
svelte: ^3.44.0 => 3.46.2

Steps to reproduce:

npm init svelte@next my-app
cd my-app
npm install
npm run dev -- --open --https

@nstuyvesant
Copy link
Contributor

nstuyvesant commented Jan 22, 2022

@joshnuss - I think you are correct as I can confirm in Chrome that the failed request was made via HTTP/2:
image
Note the h2 in the protocol column

@nstuyvesant
Copy link
Contributor

This is an acute problem for developers on macOS trying to test Safari. Safari converts requests for resources like images to HTTPS even if the href is a relative URL. The only way to successfully test a SvelteKit app on Safari on Monterey (12.1) is to use --https when running dev. Some of my app's functionality (like Apple Pay) requires HTTPS. Without it, testing is extremely limited.

@isaacfranco
Copy link
Contributor

isaacfranco commented Jan 24, 2022

I think it has to do with HTTP2, since HTTP2 headers can have colons : in them, ie :method:, :path:, but the regex used for parsing headers doesn't allow them.

That's precisely it. http2 pseudo-headers. Request can't receive headers with those.

The problem is that on getRequest(base, req), a Http2ServerRequest is converted to a Request, and headers are incompatible.

Can we can just strip those headers for now?. I don´t know if use them to infer the correct url/method of the Request object is something adequate.

diff --git a/packages/kit/src/node.js b/packages/kit/src/node.js
index 44d3ae38..ff316632 100644
--- a/packages/kit/src/node.js
+++ b/packages/kit/src/node.js
@@ -52,9 +52,14 @@ function get_raw_body(req) {
 
 /** @type {import('@sveltejs/kit/node').GetRequest} */
 export async function getRequest(base, req) {
+       const headers = Object.fromEntries(
+               Object.entries(/** @type {Record<string, string>} */ (req.headers)).filter(
+                       ([key]) => !/^:/.test(key)
+               )
+       );
        return new Request(base + req.url, {
                method: req.method,
-               headers: /** @type {Record<string, string>} */ (req.headers),
+               headers,
                body: await get_raw_body(req) // TODO stream rather than buffer
        });
 }

@joshnuss
Copy link
Author

joshnuss commented Jan 24, 2022

I'm wondering if filtering pseudo-headers will be enough. We may also need to convert the response to http2 (not sure).

Here is some info about translating HTTP2 -> HTTP1:
https://httptoolkit.tech/blog/translating-http-2-into-http-1/#translating-one-to-the-other

@isaacfranco
Copy link
Contributor

isaacfranco commented Jan 24, 2022

Hi @joshnuss. I saw that article. I'm not sure too if it is enough. But... to have a fully http2 flow I think that a lote of changes has to be made out of the scope of that particular issue. For example, the article sugest to calculate the method and URL out of pseudo-headers, but the current kit design receives that info on directily of the "req" param. Choosing to do that, we are automaticaly striping info from the original request, wich may be a problem on non http2 scenarios.

Actually I kind of did that first, but o felt a bit too much.

/** @type {import('@sveltejs/kit/node').GetRequest} */
export async function getRequest(base, req) {
	const headers = Object.fromEntries(
		Object.entries(/** @type {Record<string, string>} */ (req.headers)).filter(
			([key]) => !/^:/.test(key)
		)
	);

	//http2 pseudo-headers, even out off the header, should have priority to determine method, host and url.
	const method = /** @type {string} */(req.headers[':method']) || req.method;
	//const status = /** @type {string} */(req.headers[':status']) || req.statusCode;
	let url = base + req.url;
	if (req.headers[':scheme'] && req.headers[':authority'] && req.headers[':path']) {
		url = req.headers[':scheme'] + '://' + req.headers[':authority'] + req.headers[':path'];
	}

	return new Request(url, {
		method,
		headers,
		body: await get_raw_body(req) // TODO stream rather than buffer
	});
}

@nstuyvesant
Copy link
Contributor

Just updated to SvelteKit 1.0.0-next.241 then did a npm run -- --open --https and the error (using latest version of Chrome on macOS) changed to "Invalid request body".

@talgolan
Copy link

talgolan commented Jan 26, 2022

Just updated to SvelteKit 1.0.0-next.243 and, like @nstuyvesant, am seeing the same "Invalid request body" exception.

This is a hard blocker for us as https development is required for our application(s).

Is there any guidance/workaround at this time? At this point we are frozen at:

  • "@sveltejs/kit": "1.0.0-next.233"
  • "@sveltejs/adapter-node": "1.0.0-next.67"

Thank you.

@Conduitry
Copy link
Member

I've opened #3553 to hopefully address this. If people could take a look at that and confirm whether that fixes their issue, that would be appreciated.

@Conduitry
Copy link
Member

In the meantime, or if you don't want to try using a local version of SvelteKit, you should be able to work around this by setting kit.vite.server.proxy to {} in your svelte.config.js.

@talgolan
Copy link

talgolan commented Jan 26, 2022

@Conduitry: Thank you. Setting kit.vite.server.proxy = {} in our svelte.config.js has allowed us to upgrade to:

  • "@sveltejs/adapter-node": "1.0.0-next.67"
  • "@sveltejs/kit": "1.0.0-next.243"

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

Successfully merging a pull request may close this issue.

7 participants