http-server icon indicating copy to clipboard operation
http-server copied to clipboard

Added option for http response headers

Open hedefalk opened this issue 9 years ago • 34 comments

Use case was: https://github.com/indexzero/http-server/issues/281

Feel free to close if doesn't seem relevant…

Fixes #174, fixes #334, fixes #644

hedefalk avatar May 26 '16 14:05 hedefalk

Hello, I just comment to say it seems totally relevant to me, I would really appreciate if it got merged.

ssimono avatar Jun 06 '16 16:06 ssimono

This is super useful. Someone please get it merged and do a publish. I could use this right now

nojvek avatar Jun 10 '16 01:06 nojvek

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 )

howardroark avatar Jun 22 '16 15:06 howardroark

Subscribed, I think this would allow me to set AMP-Access-Control-Allow-Source-Origin to develop https://www.ampproject.org projects.

JamieMason avatar Jan 05 '17 11:01 JamieMason

👍 This would also useful for setting/testing Content Security Policy headings.

BigBlueHat avatar Feb 21 '17 16:02 BigBlueHat

@indexzero any plan on merging this?

jaylinski avatar Oct 04 '17 14:10 jaylinski

@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 😄

coderkd10 avatar Feb 15 '18 07:02 coderkd10

Only 5 years have passed...

Tofandel avatar Jul 23 '21 16:07 Tofandel

minimist allows the parsing of 1. Objects and 2. Multiple options of the same name.

Thus, this would work with minimal code addition:

http-server --headers.AMP-Access-Control-Allow-Source-Origin '[email protected]' --headers.Cookie 'test=1'

▶️ Run this code in Runkit.

The argv.headers would be equal to:

{
  'AMP-Access-Control-Allow-Source-Origin': '[email protected]',
  'Cookie': 'test=1'
}

Techically, that's the only thing to pass to the options object, as the httpServer already handles the (undocumented) headers option.

thom4parisot avatar Aug 16 '21 11:08 thom4parisot

@oncletom you're totally right about minimist supporting object options like that, however, I do like the PR's implementation, which makes an array, a little better because it passes the headers as a single string, which seems more natural to me.

thornjad avatar Aug 16 '21 20:08 thornjad

I’m game on providing the requested changes if @hedefalk somewhat cannot manage to do so.

thom4parisot avatar Aug 18 '21 08:08 thom4parisot

@oncletom would you like to make the requested changes? Otherwise I can take this over

thornjad avatar Oct 12 '21 03:10 thornjad

such a simple addition, surprised it's missing, wanted to add Cross-Origin-Resource-Policy to https://developer.chrome.com/blog/enabling-shared-array-buffer/#cross-origin-isolation

hlolli avatar Nov 15 '21 05:11 hlolli

Would love to see this added.

rgeraldporter avatar Jan 20 '22 15:01 rgeraldporter

is custom response headers supported?

cwtuan avatar Feb 09 '22 07:02 cwtuan

would this allow custom headers for files matching a regex?

-H *.js.gz="Content-Type=text/javascript"

JulianIsrael avatar Feb 21 '22 12:02 JulianIsrael

@thornjad would it be possible to get this landed still?

Pomax avatar Apr 25 '22 14:04 Pomax

Is it dead?

josenicomaia avatar Jun 13 '22 07:06 josenicomaia

Is this the sign for us to choose another package ?

BelgianNoise avatar Aug 25 '22 12:08 BelgianNoise

@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 avatar Aug 25 '22 14:08 BigBlueHat

@BigBlueHat thanks for confirming that you're still interested in this project. Will you be ale to merge an release https://github.com/http-party/http-server/pull/811 ? (sorry for cross-posting, but that issue seems to be abandoned by maintainers)

zbynek avatar Aug 25 '22 15:08 zbynek

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, you can provide a mime type file to the server using the --mimetypes option https://github.com/http-party/http-server/blob/master/test/public/custom_mime_type.types https://github.com/http-party/http-server/blob/93fbb755fe55aa13878af9a91bc8e1a274522dbe/test/cli.test.js#L64

As it was established, this PR is still missing a test case and documentation

Nice to know the project is still alive though

Tofandel avatar Aug 25 '22 19:08 Tofandel

helping the core contributors know if this patch is what's needed or if more work remains

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 docs/http-server.1

.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:

--- 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.| |

alexanderankin avatar Aug 28 '22 21:08 alexanderankin

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);

alexanderankin avatar Aug 28 '22 22:08 alexanderankin

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?

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.

Pomax avatar Aug 29 '22 15:08 Pomax

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.

jonathanding avatar Nov 08 '22 12:11 jonathanding

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.

64jcl avatar Feb 13 '23 14:02 64jcl

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 avatar Feb 15 '23 12:02 iliakan

@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:

  1. find where the bin file is located on your system which http-server
  2. to get your basepath edit bin file nano [your-path]
  3. add this echo to print out your basepath
#!/bin/sh
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
echo $basedir
  1. open the directory with your editor
code [your-basepath]../pnpm-global/5/node_modules/http-server/bin/http-server
  1. copy the changes from files

ddresch avatar Mar 04 '23 05:03 ddresch

  1. Install https://www.npmjs.com/package/patch-package and run it on the http-server

Tofandel avatar Mar 04 '23 09:03 Tofandel