-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Added option for http response headers #282
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
Conversation
Hello, I just comment to say it seems totally relevant to me, I would really appreciate if it got merged. |
This is super useful. Someone please get it merged and do a publish. I could use this right now |
Yeah I think I could use this over what I submitted for #288 :) ( Maybe it could use a few more "thumbs up" reactions ... http://gitsup.com/indexzero/http-server :P ) |
Subscribed, I think this would allow me to set |
👍 This would also useful for setting/testing Content Security Policy headings. |
@indexzero any plan on merging this? |
@indexzero @BigBlueHat gentle reminder. it will be super helpful if you could merge this PR. Also let me know if any changes are required. I'll be happy to help 😄 |
Only 5 years have passed... |
Thus, this would work with minimal code addition: http-server --headers.AMP-Access-Control-Allow-Source-Origin '[email protected]' --headers.Cookie 'test=1' The {
'AMP-Access-Control-Allow-Source-Origin': '[email protected]',
'Cookie': 'test=1'
} Techically, that's the only thing to pass to the |
@oncletom you're totally right about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the simplicity! Here's what we still need before merging:
- Add documentation to doc/http-server.1
- Add documentation to README.md
- Add tests (at least one for a single extra header and one for multiple extra headers)
I’m game on providing the requested changes if @hedefalk somewhat cannot manage to do so. |
@oncletom would you like to make the requested changes? Otherwise I can take this over |
Would love to see this added. |
is custom response headers supported? |
would this allow custom headers for files matching a regex?
|
@thornjad would it be possible to get this landed still? |
Is it dead? |
Is this the sign for us to choose another package ? |
@josenicomaia @BelgianNoise the best way to help any OSS project is to contribute. In this case, testing this PR, confirming that it works, explaining what you tested, etc. would go along way to helping the core contributors know if this patch is what's needed or if more work remains. @cwtuan @JulianIsrael if you're curious about specific capabilities, please also take the time to test this branch, find out for yourselves, and provide confirmation here that it does support those things. Community projects take a community. |
@BigBlueHat thanks for confirming that you're still interested in this project. Will you be ale to merge an release #811 ? (sorry for cross-posting, but that issue seems to be abandoned by maintainers) |
To answer @cwtuan that's literally the point of the PR so I don't get the question To answer @JulianIsrael nope it wouldn't, there is nothing in this PR that would do that, but what you want to do already exists, Line 64 in 93fbb75
As it was established, this PR is still missing a test case and documentation Nice to know the project is still alive though |
a PR being submitted usually implies that the submitter just wants to merge the changes supplied (unless explicitly noted otherwise). Open Source maintainers aren't supposed to be HR managers who sign off on things unknowingly? as far as the requested changes, here's what I'd add to .TP
.BI \-H ", " \-\-header " " \fIHEADER\fR
Add extra header to all responses, eg. "X-Frame-Options: DENY". (right after cors): --- man.1 2022-08-28 17:38:33.244404883 -0400
+++ man-before 2022-08-28 17:36:54.973302056 -0400
@@ -63,10 +63,6 @@
Optionally provide CORS headers list separated by commas.
.TP
-.BI \-\-H ", " \-\-header " " \fIHEADER\fR
-Add extra header to all responses, eg. "X-Frame-Options: DENY".
-
-.TP
.BI \-o " " [\fIPATH\fR]
Open default browser window after starting the server.
Optionally provide a URL path to open the browser window to. and to --- README.md 2022-08-28 17:40:21.619418290 -0400
+++ README.md-after 2022-08-28 17:41:32.794771710 -0400
@@ -53,6 +53,7 @@
|`-e` or `--ext` |Default file extension if none supplied |`html` |
|`-s` or `--silent` |Suppress log messages from output | |
|`--cors` |Enable CORS via the `Access-Control-Allow-Origin` header | |
+|`-H` or `--header` |Add extra header to all responses, eg. `X-Frame-Options: DENY`| |
|`-o [path]` |Open browser window after starting the server. Optionally provide a URL path to open. e.g.: -o /other/dir/ | |
|`-c` |Set cache time (in seconds) for cache-control max-age header, e.g. `-c10` for 10 seconds. To disable caching, use `-c-1`.|`3600` |
|`-U` or `--utc` |Use UTC time format in log messages.| | |
here are tests: diff --git a/test/http-server-test.js b/test/http-server-test.js
index d4555b9..d3c7f13 100644
--- a/test/http-server-test.js
+++ b/test/http-server-test.js
@@ -154,5 +154,48 @@ vows.describe('http-server').addBatch({
assert.ok(res.headers['access-control-allow-headers'].split(/\s*,\s*/g).indexOf('X-Test') >= 0, 204);
}
}
+ },
+ 'When extra header are provided': {
+ topic: function () {
+ var server = httpServer.createServer({
+ root: root,
+ extraHeaders: ['X-Header:extra']
+ });
+ server.listen(8083);
+ this.callback(null, server);
+ },
+ 'and request is issued to server with extra header': {
+ topic: function () {
+ request({
+ method: 'GET',
+ uri: 'http://127.0.0.1:8083/'
+ }, this.callback);
+ },
+ 'response should contain extra header': function (err, res) {
+ assert.equal(res.headers['x-header'], 'extra');
+ }
+ }
+ },
+ 'When multiple extra headers are provided': {
+ topic: function () {
+ var server = httpServer.createServer({
+ root: root,
+ extraHeaders: ['X-Header-One:extra-one', 'X-Header-Two:extra-two']
+ });
+ server.listen(8084);
+ this.callback(null, server);
+ },
+ 'and request is issued to server with extra headers': {
+ topic: function () {
+ request({
+ method: 'GET',
+ uri: 'http://127.0.0.1:8084/'
+ }, this.callback);
+ },
+ 'response should contain extra headers': function (err, res) {
+ assert.equal(res.headers['x-header-one'], 'extra-one');
+ assert.equal(res.headers['x-header-two'], 'extra-two');
+ }
+ }
}
}).export(module); |
This PR addresses an open issue, or rather: four issues; it's not a drive-by PR. The discussion about what the PR should do goes in those issues, and the discussion in the PR itself should be about whether the code addresses the need/fix/functionality those issues need in order to be considered resolved, as well as whether the PR's code requires changes before it can be merged or not. A PR being submitted does not imply someone "just wants to merge the changes", it implies they took on the task of fixing an issue and they are now submitting that work for review because they believe it's ready for that. This absolutely requires review and sign-off from the people who actually maintain the project, since it's their project: it's their responsibility to accept contributions or not (and if so, how much review they deem necessary before accepting). And on a practical, github-using note in terms of comments with diffs: if these are changes to files already in the PR, just use the PR commenting system instead (i.e. find the file and line(s) in the PR you flagged as needing more work, click the comment button next to th(os)e line number(s) and leave a comment with "code suggestion" that the PR owner can then automatically accept into the PR) or if it's a completely new change to a file not already in the PR, hit up their branch, click "edit" on that file, let github do its forking magic if necessary, and then file your own PR against their PR. This can all be done without ever leaving github, by taking advantage of the tooling that's already in place for reviews and edits. |
Is there any update for this feature? This seems to be very useful and surprisingly to see it is missing and takes so long to get merged. |
Was surprised there was no way to add custom headers to the response of http-server. I need it in order to add "Cross-Origin-Opener-Policy: same-origin" and "Cross-Origin-Embedder-Policy: require-corp" in order to get SharedArrayBuffer to work. |
Exactly the same thing! There's no support yet? |
@iliakan if you are still struggling. Just merge the changes of the two files yourself. ATTENTION: not update safe – but anyway if just want to go on :) Here is the process I used for PNPM:
#!/bin/sh
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
echo $basedir
|
|
Out of desperation I've forked all this and merged hedefalk's 7 year old version with the latest stuff here. It's a temporary hack solution and I'm likely not going to merge future changes into that, so it's officially unmaintained, but it's a workaround for the desperate like me until the authors merge in hedefalk's changes. |
For those still waiting on custom headers, I have been rewriting |
alternatively, most of my projects just use a copy-paste import express from "express";
const PORT = process.env.PORT ?? 8000;
process.env.PORT = PORT;
const HOSTNAME = process.env.HOSTNAME ?? `localhost`;
process.env.HOSTNAME = HOSTNAME;
const app = express();
// no caching because this is a dev server.
app.use((req, res, next) => {
res.setHeader("Surrogate-Control", "no-store");
res.setHeader(
"Cache-Control",
"no-store, no-cache, must-revalidate, proxy-revalidate"
);
res.setHeader("Expires", "0");
next();
});
// no etags, because again, this is a dev server
app.set("etag", false);
// route call logging, because... you get the idea.
app.use((req, res, next) => {
console.log(`[${new Date().toISOString()}] ${req.url}`);
next();
});
// The entire directory, please.
app.use(`/`, express.static(`.`));
// And if it's not there, just yell about that.
app.use((req, res) => {
console.log(`[${new Date().toISOString()}] Could not find ${req.url}`);
res.status(404).send(`${req.url} not found`);
});
// Run the server
app.listen(PORT, () => {
const msg = `= Server running on http://${HOSTNAME}:${PORT} =`;
const line = `=`.repeat(msg.length);
const mid = `=${` `.repeat(msg.length - 2)}=`;
console.log([``, line, mid, msg, mid, line, ``].join(`\n`));
}); Done, |
Also https://www.npmjs.com/package/serve is a good replacement for this package. |
Probably better to leave this open until one of those actually lands, given how little has happened in this project. I'll be happy to see either of those land, but so far the track record has been "that's not happening", and both of those are PRs, not an issue, so they're not the right place for folks to (keep) leave('ing) comments =) |
Newer PRs are easier to validate and merge, so this seems like reasonable decision to me. This thread you replied on is also a PR; issue #884 tracks this. I recently (today) became a maintainer which hopefully means I'll do a fantastic job at it and bring life back into the PRs - time will tell! |
I see! In that case, good luck, good to hear someone's back in the maintainer seat. |
Anyone still following this may be interested to participate in discussion #899. Current plan is to include custom headers in the next release, and a PR adding this has been merged. |
Holy shit, what an adventure. Thank you so much! Been using a fork, now I can finally use official again. |
Use case was: #281
Feel free to close if doesn't seem relevant…
Fixes #174, fixes #334, fixes #644