Skip to content
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

Unable to remove response header #206

Closed
SDrinkwater opened this issue Sep 1, 2020 · 10 comments · Fixed by #223
Closed

Unable to remove response header #206

SDrinkwater opened this issue Sep 1, 2020 · 10 comments · Fixed by #223
Assignees

Comments

@SDrinkwater
Copy link

SDrinkwater commented Sep 1, 2020

There doesn't seem to be a way to remove a response header. I am trying to remove the X-Powered-By header. I am only able to set its value, through which I can effectively achieve the same result, but it would be nice to be able to actually remove the header.

Is there some way to achieve this?

Issue originally raised here: firebase/firebase-functions#754

@grant
Copy link
Contributor

grant commented Sep 8, 2020

Hi @SDrinkwater,
Sorry, I don't understand your question. With res.send( in Express, you can send any response.

https://expressjs.com/en/api.html#res.send

The X-Powered-By is not coming from this library.

@rhodgkins
Copy link

rhodgkins commented Sep 15, 2020

I'd also like to see this functionality added as we are also trying to remove the x-powered-by header.

@grant I think this header is coming from this library as it creates the express app:

const app = express();

The default setting for x-powered-by is true which means the value of the header will be Express (see the docs).

Then any sub-apps that are used for client function routes have their x-powered-by setting ignore.

You can try it with the following:

const app = express()
app.get('/', (req, res) => {
   res.send('Will have x-powered-by header')
})    

const subapp = express()
subapp.disable('x-powered-by')
subapp.all('*', (req, res) => {
   res.send(`Will still have x-powered-by header despite being disabled: subapp.enabled('x-powered-by')=${subapp.enabled('x-powered-by')}`)
})
app.use('/subapp', subapp)

// Matches how functions are called
app.all('/calledsubapp', (req, res, next) => {
   subapp(req, res, next)
})

Querying the main app:

curl -i http://localhost:3000       
HTTP/1.1 200 OK
X-Powered-By: Express
Content-Type: text/html; charset=utf-8
Content-Length: 29
ETag: W/"1d-6q73dA/19m+t4qsFOySdg9LHLZI"
Date: Tue, 15 Sep 2020 08:09:26 GMT
Connection: keep-alive

Will have x-powered-by header     

Querying the sub app:

curl -i http://localhost:3000/subapp
HTTP/1.1 200 OK
X-Powered-By: Express
Content-Type: text/html; charset=utf-8
Content-Length: 96
ETag: W/"60-FbpOzXvracH4MnF2RPc157pCkPE"
Date: Tue, 15 Sep 2020 08:10:35 GMT
Connection: keep-alive

Will still have x-powered-by header despite being disabled: subapp.enabled('x-powered-by')=false

Querying the called sub app:

curl -i http://localhost:3000/calledsubapp
HTTP/1.1 200 OK
X-Powered-By: Express
Content-Type: text/html; charset=utf-8
Content-Length: 96
ETag: W/"60-FbpOzXvracH4MnF2RPc157pCkPE"
Date: Tue, 15 Sep 2020 08:17:51 GMT
Connection: keep-alive

Will still have x-powered-by header despite being disabled: subapp.enabled('x-powered-by')=false

Adding in app.disable('x-powered-by') removes the header for all three cases.

Also the docs even explicitly recommend disabling this header.

@rhodgkins
Copy link

Happy to do a PR (with some tests) to at least disable the x-powered-by header...

@grant
Copy link
Contributor

grant commented Sep 15, 2020

Oh, I don't think this header is intentionally added.

A PR with tests to disable the header for all functions would be great. @rhodgkins

It looks like we should add helmet. https://www.npmjs.com/package/helmet#how-it-works
I can create a PR.

@grant
Copy link
Contributor

grant commented Sep 21, 2020

Hi folks,
Above or here is a sample where you can use helmet.
#214 (comment)

LMK if there are other issues with removing that response header.

@grant grant closed this as completed Sep 21, 2020
@rhodgkins
Copy link

rhodgkins commented Sep 22, 2020

This issue should not be closed or at least another issue about x-powered-by should be opened.

Even using helmet my above reproduction steps still apply for that header as it’s being set at a level where the end function user can’t reach.

@SDrinkwater
Copy link
Author

@grant Can we please reopen this issue. The comment you linked #214 (comment) doesn't solve the problem, it just suggests why helmet shouldn't be installed by default.

The problem really comes down the fact that we don't have access to the root app so are unable to override settings on it.

@grant
Copy link
Contributor

grant commented Sep 25, 2020

@SDrinkwater Sure, I can re-open.

Did exporting an express app with helmet middleware not solve the issue? That should intercept all requests and is essentially the root app (the root app routes all requests to your app). And app.disable('x-powered-by'); should disable the header.

Can you provide reproduction steps and expected/actual behavior you are seeing with code?

@grant grant reopened this Sep 25, 2020
@SDrinkwater
Copy link
Author

@grant Alright, looks like I might have jumped the gun. Adding helmet to an express function does remove the header, as does the following code:

const app = express();
app.use((_req, res, next) => {
  res.removeHeader("X-Powered-By");
  next();
});
app.use(testFunction);

function testFunction(req, res) {
  res.send("Hello World");
}

exports.testFunction = app;

However, I originally raised this issue as a suggestion from another very similar issue raised in the firebase/firebase-functions repo firebase/firebase-functions#754.

This library is apparently the underlying framework used by firebase cloud functions and hence the question posted here.

Using the above technique doesn't remove the header when used with firebase-functions https.onRequest, so there has to be some kind of handling in between somewhere. Happy to take the problem back to that other repo, unless there is some way you can coordinate with them?

@grant
Copy link
Contributor

grant commented Sep 27, 2020

Thanks for working through this.

If the only ask is to remove the header, I think adding a line to remove it wouldn't be contested:

app.disable('x-powered-by');

Here:

// Apply middleware

I think there was some contention with adding helmet, but just removing that header should be fine.

Previous thoughts

I don't mean to juggle the issue, but if there is a way in this repo/module to remove the header, but a consumer (Firebase Functions) doesn't support this solution, that sounds like it should be triaged with Firebase Functions.

The Functions Framework supports Express middleware so you can export const app = <express object>. So using helmet or just the direct middleware should work.

In the PR, we talked about adding Helmet, but we're sure if we wanted to add every feature and if all users wanted that #214 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants