Skip to content

Conversation

jimlawhorn
Copy link

Add --headers argument support and update doco

@jimlawhorn jimlawhorn force-pushed the add_header_support branch 2 times, most recently from 81fdaf7 to 6f30208 Compare August 5, 2018 00:16
Add --headers argument support and update doco
@BigBlueHat
Copy link
Member

Thanks for sending in a PR!

Given that this will add the same header to ever request, what's the use case for this?

@jimlawhorn
Copy link
Author

My specific use case was for app development. I'm using http-server to test/develop API consumption against. I have vendor provided apps that provide some information in the headers that are necessary to consume the response data correctly. Rather than go through the process of setting up the data in the system (which I can't necessarily do), I just have saved responses that I serve up with http-server. I had been using Fiddler Autoresponder to inject the headers and http-server, but it made more sense to me to just add it as an option to the http-server.

From a non-development standpoint, I didn't see any way to set the Access-Control-Allow-Methods, Content-Security-Policy, or some of the more common standard X-* headers. I don't know if anyone is using for production settings, but I figured if there was a --cors option, someone is at least testing with it. These are all common headers that we have in the web server, rather than code.

@BigBlueHat
Copy link
Member

Completely agree that header customization is needed, but the scenarios where one would add the same header to all requests seem few. Take a look at #360 for something I'd like to get added at some point. It would allow per-file (at least) header customization via a .headers file (which is a convention...sadly not yet a standard) used by other projects. Your thoughts and help here would be most welcome!

@jimlawhorn
Copy link
Author

The .headers idea is significantly better in a lot of ways and if done correctly, would be easy enough to use for static headers (like I need). Unless you think there's value to this PR in addition to that functionality, I'm going to close this PR and take a crack at the headers idea.

@jimlawhorn jimlawhorn closed this Aug 8, 2018
@BigBlueHat
Copy link
Member

FWIW, it looks like the underlying node-ecstatic does support a --H option for site-wide header usage: https://github.com/jfhbrook/node-ecstatic#optsheaders

Consequently, it probably makes sense to expose that option via http-server after all.

I'd still love .headers made available (and perhaps upstreamed to node-ecstatic if they're interested).

Balancing flexibility and focus is tricky. Input welcome!

@BigBlueHat BigBlueHat reopened this Aug 9, 2018
@numical
Copy link

numical commented Oct 4, 2018

+1 from me.
My use case is using http-server in dev and local testing to emulate a front-end proxy that is adding custom HTTP headers that my client-side app reads.

@BigBlueHat
Copy link
Member

@numical would you like to take a crack at a PR for #360?

For this PR specifically, I'd like to rename the --headers switch to match ecstatic's (and curl's) -H switch naming because it's shorter and more common among similar apps.

@jimlawhorn would you be up for tackling the above tweak?

@numical
Copy link

numical commented Oct 4, 2018

Cannot commit at the moment - sorry.
Struggling even to keep my projects up to date.
However, if I get a chance, will take a look. Just pls don't wait for me!

@numical
Copy link

numical commented Oct 5, 2018

@BigBlueHat - I have got as far as forking the repo and taking a look. Looks interesting if I can find the time! In the meantime could you accept this @jimlawhorn's current PR? It would satisfy my current needs.

@jimlawhorn
Copy link
Author

I'm looking at updating the original PR.

@jimlawhorn
Copy link
Author

@numical would you like to take a crack at a PR for #360?

For this PR specifically, I'd like to rename the --headers switch to match ecstatic's (and curl's) -H switch naming because it's shorter and more common among similar apps.

@jimlawhorn would you be up for tackling the above tweak?

Submitted updates for the static header options. I opened #460 a while ago for the dynamic headers. Not seen any feedback on it, but I'd love any comments.

@BigBlueHat
Copy link
Member

@jimlawhorn this is looking good! Could you add some tests to handle the new feature?

Also, it might be good to make "H" the alias, so the code reads more clearly (i.e. argv.H looks odd...).

Thanks!

@thornjad thornjad self-requested a review April 17, 2019 14:55
@thornjad thornjad added this to the Custom headers milestone Aug 16, 2021
@thornjad
Copy link
Member

Closing in favor of the simpler and older #282

@thornjad thornjad closed this Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants