http: add `http.createStaticServer` #45096

aduh95 · 2022-10-20T16:39:40Z

This PR adds a small utility to spawns a really simple HTTP 1.1 server to serve a local directory. The server is not meant to be secure or extremely performant, it's meant to be minimal and a useful replacement for python -m http.server. This should never be used in production of course.

Usage:

node -e 'http.createStaticServer()' # starts serving the cwd on a random port

# To start serving on the port 8080 using /path/to/dir as the root:
node -e 'http.createStaticServer({directory: "/path/to/dir", port: 8080})'

# Same as above, but exposing your local file system to the whole IPv4 network:
node -e 'http.createStaticServer({directory: "/path/to/dir", port: 8080, host: "0.0.0.0"})'

~~Semver-major if we want to release it without requiring the node: prefix.~~
EDIT: I've removed the node:http/static module from this PR, it can be discussed in a separate PR.

Fixes: #45079

doc/api/http.md

jasnell · 2022-10-21T13:55:38Z

doc/api/http.md

+node -r node:http/server # starts serving the cwd on a random port
+
+# To start serving on the port 8080 using /path/to/dir as the root:
+node -r node:http/server /path/to/dir --port 8080


Not really a fan of building in a preload like this. Someone can easily use the -pe syntax to achieve this if they wanted, e.g. node -pe "require('http').createStaticServer(...)

One reason to use -r is you can get autocomplete when selecting the root directory, and it's easier to offer shorter versions of flags (i.e. I'd rather type node -r http/server . -p 8080 -h 0.0.0.0 than node -e 'http.createStaticServer({port: 8080, host: '0.0.0.0'})'). Currently this PR offers both syntax, so I guess I should also document that, and folks can chose the syntax they prefer, wdyt?

lib/http/server.js

jasnell · 2022-10-21T13:58:35Z

lib/http/server.js

+
+emitExperimentalWarning('http/server');
+
+if (module.isPreloading) {


This is dangerous. If someone has their own preload module and happens to require('http/server') within it, this part will be run while they may not expect it.

lib/http/server.js

mcollina

This needs tests.

I would also recommend we implement:

partial responses (Ranges)
conditional-GET negotiation (If-Match, If-Unmodified-Since, If-None-Match, If-Modified-Since) - this includes etag generation.

It's not necessary that those are implemented now, however we should note those are not implemented in the docs.

mcollina · 2022-10-22T08:43:16Z

lib/http/server.js

+          stream.on('error', reject);
+          stream.on('close', resolve);
+          stream.pipe(res);
+        });


You should use stream.pipeline instead if this.

pipeline implementation seems overly complicated in comparison of what's needed here (we don't need any AbortSignal support, etc.), and tries to do more that we want (i.e. it closes the request in case of error before we can send a 404 status).

doc/api/http.md

aduh95 · 2022-10-23T02:50:55Z

Tests have been added, this is ready for reviews. There's a discussion regarding the name of the module (http/server, or http/static, or?) in #45096 (comment), if you have thoughts please chime in.

aduh95 · 2022-10-23T16:00:23Z

There are a few CI failures that need to be taken care of in separate PRs, converting this one back to draft.

jasnell · 2022-10-26T01:19:12Z

lib/http/static.js

+const { validateFunction, validatePort } = require('internal/validators');
+
+if (module.isPreloading) {
+  const requiredModules = getOptionValue('--require');


Still very much not a fan of building preload into this.

Keep in mind, for instance, that -r is permitted in the NODE_OPTIONS

Do you have in mind what changes we would want to bring before we're happy with this pattern? Or do you think the pattern itself is undesirable? Personally I quite like the UX of it, and I think it's better than coming up with another built-in flag, but I'm happy to withdraw it if I'm in the minority.

lib/http/static.js

aduh95 · 2022-11-23T13:19:46Z

@mcollina are you still blocking this?

mcollina

lgtm

…y default

Co-authored-by: Michaël Zasso <targos@protonmail.com> Co-authored-by: Tobias Nießen <tniessen@tnie.de>

vlourenco-stf

Non-blocking comments, just things to make it a little bit faster.

lib/internal/http/static.js

vlourenco-stf · 2024-04-03T13:14:24Z

lib/internal/http/static.js

+
+    try {
+      try {
+        const stream = createReadStream(url, { emitClose: false });


I know is a nitpick but I think you can cache this object to avoid creating every time.

Also, maybe expose the highWaterMark to the user configure is a good idea.

I'm not sure I understand what you mean, do you mean cache the stream? We're going to need a different stream for each request, I think, so I don't see how it'd be useful 🤔 could you send an example maybe?

Caching { emitClose: false }

ruyadorno

LGTM 👍

github-actions · 2024-04-04T13:06:36Z

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @ruyadorno.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

tniessen · 2024-04-04T13:07:42Z

Should there be a note about security in the docs? Specifically, I am wondering what would constitute a vulnerability here.

ruyadorno · 2024-04-04T13:07:44Z

@aduh95 heads up, please update the usage example in the body so that releasers & other collaborators can potentially refer to it.

aduh95 added the semver-major PRs that contain breaking changes and should be released in the next major version. label Oct 20, 2022

nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Oct 20, 2022

aduh95 mentioned this pull request Oct 20, 2022

feat: Implement a way to expose static files from a directory (similar to Python SimpleHttpServer) #45079

Open

targos reviewed Oct 21, 2022

View reviewed changes

doc/api/http.md Outdated Show resolved Hide resolved

VoltrexKeyva reviewed Oct 21, 2022

View reviewed changes

doc/api/http.md Outdated Show resolved Hide resolved

doc/api/http.md Outdated Show resolved Hide resolved

doc/api/http.md Outdated Show resolved Hide resolved

doc/api/http.md Outdated Show resolved Hide resolved

doc/api/http.md Outdated Show resolved Hide resolved