Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mighty-wasps-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/adapter-node': minor
---

feat: add opt-in HTTPS and HTTP/2 support
14 changes: 14 additions & 0 deletions documentation/docs/25-build-and-deploy/40-adapter-node.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,20 @@ We instead read from the _right_, accounting for the number of trusted proxies.

The maximum request body size to accept in bytes including while streaming. Defaults to 512kb. You can disable this option with a value of 0 and implement a custom check in [`handle`](hooks#server-hooks-handle) if you need something more advanced.

### `CERT_PATH`, `CERT_KEY_PATH`, and `HTTPS_PORT`:
By default, the server only listens for plain HTTP requests. To enable an additional HTTPS endpoint, you must provide the `CERT_PATH` and `CERT_KEY_PATH` environment variables pointing to a file containing a valid certificate and a file containing its associated private key respectively, both in the PEM format.
The files are read once on startup, so if the contents of the files change, the app will need to be restarted.

The default port that will be used for the HTTPS endpoint is 3001. You can customize this via the `HTTPS_PORT` environment variable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> We recommend using a reverse proxy instead and only use these options when you have no other possibilities of securing the connection

Copy link
Contributor Author

@aradalvand aradalvand Jun 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a slightly reworded version of this in 083af9a

@dummdidumm Also, one little question: I just noticed that variables like export const cert_path = env('CERT_PATH', false); are actually exported, so according to the style guide they should be camelCase, am I right?

  • Internal variables are written with snake_case while external APIs are written with camelCase

Do these count as "external APIs"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they should be camel case here yes, good catch

Copy link
Member

@benmccann benmccann Jun 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally I think the recommendation should be in the opposite direction. this seems better than a reverse proxy to me as I explained here: #10183 (comment)

perhaps we should just remove this section if we can't agree on a recommendation? or perhaps we can list pros and cons rather than just making a blanket statement?

> We generally recommend you use a reverse proxy in front of your application to enable HTTPS instead, and have your reverse proxy communicate with your SvelteKit application via plain HTTP; only use these options when you know what you're doing, or as a last resort.

### `NO_HTTP2`:
By default, the HTTPS endpoint will support HTTP/2 as well as HTTP/1.1. If, for some reason, supporting HTTP/2 isn't desirable, you can disable it by setting the `NO_HTTP2` environment variable to a non-empty value (e.g. `1`).

### `ONLY_HTTPS`:
If you only need an HTTPS endpoint and want to opt out of the default plain HTTP one, you can do so by setting the `ONLY_HTTPS` environment variable to a non-empty value (e.g. `1`).

## Options

The adapter can be configured with various options:
Expand Down
7 changes: 6 additions & 1 deletion packages/adapter-node/src/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ const expected = new Set([
'ADDRESS_HEADER',
'PROTOCOL_HEADER',
'HOST_HEADER',
'BODY_SIZE_LIMIT'
'BODY_SIZE_LIMIT',
'CERT_PATH',
'CERT_KEY_PATH',
'HTTPS_PORT',
'ONLY_HTTPS',
'NO_HTTP2'
]);

if (ENV_PREFIX) {
Expand Down
37 changes: 32 additions & 5 deletions packages/adapter-node/src/index.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,42 @@
import { handler } from 'HANDLER';
import { env } from 'ENV';
import polka from 'polka';
import http from 'node:http';
import https from 'node:https';
import http2 from 'node:http2';
import fs from 'node:fs';

export const path = env('SOCKET_PATH', false);
export const host = env('HOST', '0.0.0.0');
export const port = env('PORT', !path && '3000');
export const certPath = env('CERT_PATH', false);
export const certKeyPath = env('CERT_KEY_PATH', false);
export const httpsPort = env('HTTPS_PORT', !path && '3001');
export const onlyHttps = env('ONLY_HTTPS', false);
export const noHttp2 = env('NO_HTTP2', false);

const server = polka().use(handler);
const app = polka().use(handler);

server.listen({ path, host, port }, () => {
console.log(`Listening on ${path ? path : host + ':' + port}`);
});
if (!onlyHttps) {
// TODO Remove the `@ts-expect-error`s below once https://github.com/lukeed/polka/issues/194 is fixed
// @ts-expect-error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure you can bypass the ts-expect-errors by creating server via native modules (as you are) and then passing through options.server to Polka. The type errors are there cuz of Node's complex types.

let server = certPath && certKeyPath
  ? (noHttp2 ? https.createServer({ ... }) : http2.createServer({ ... })
  : http.createServer();

let app = polka({ server });

Copy link
Contributor Author

@aradalvand aradalvand Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukeed This was brought up and I explained why I decided against it here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw but it's invalid because there's only 1 Polka instance & 1 server needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukeed Not that it matters anymore but that's incorrect, you could have 2 servers, a plain HTTP one and an HTTPS one; hence the intention to reuse the Polka instance.

http.createServer(app.handler).listen({ path, host, port }, () => {
console.log(`Listening on http://${path ? path : host + ':' + port}`);
});
}

export { server };
if (certPath && certKeyPath) {
const cert = fs.readFileSync(certPath);
const key = fs.readFileSync(certKeyPath);
const https_server = noHttp2
? // @ts-expect-error
https.createServer({ cert, key }, app.handler)
: // @ts-expect-error
http2.createSecureServer({ allowHTTP1: true, cert, key }, app.handler);

https_server.listen({ path, host, port: httpsPort }, () => {
console.log(`Listening on https://${path ? path : host + ':' + httpsPort}`);
});
}

export { app as server };