Skip to content

Add X-Powered-By header. #416

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Dec 19, 2016
Merged

Add X-Powered-By header. #416

merged 4 commits into from
Dec 19, 2016

Conversation

arunoda
Copy link
Contributor

@arunoda arunoda commented Dec 17, 2016

See:

sh-3.2$ curl -v http://localhost:3000
* Rebuilt URL to: http://localhost:3000/
*   Trying ::1...
* Connected to localhost (::1) port 3000 (#0)
> GET / HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.49.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: text/html
< Content-Length: 3172
-------------------------------
< X-Powered-By: Next.js 1.2.3
-------------------------------
< Date: Sat, 17 Dec 2016 11:38:52 GMT
< Connection: keep-alive

@coveralls
Copy link

coveralls commented Dec 17, 2016

Coverage Status

Coverage increased (+0.01%) to 57.143% when pulling 948c348 on arunoda:x-powered-by into b62a0e8 on zeit:master.

@STRML
Copy link

STRML commented Dec 17, 2016

This feels like a bad idea - in general you don't want to leak information about the server in any production app - the framework name is bad enough, the exact version is worse because it makes it that much easier to target exploits.

@@ -102,6 +104,7 @@ export async function renderErrorJSON (err, res, { dir = process.cwd(), dev = fa
export function sendHTML (res, html) {
res.setHeader('Content-Type', 'text/html')
res.setHeader('Content-Length', Buffer.byteLength(html))
res.setHeader('X-Powered-By', `Next.js ${pkg.version}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use pkg.name to DRY

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it only says next. Not Next.js.

@jamo
Copy link
Contributor

jamo commented Dec 17, 2016

Yeah, +1 for obfuscating the version. Tho if the page downloads next.js/<version>/next.min.js it's pretty obvious which version and framework is being used...

@arunoda
Copy link
Contributor Author

arunoda commented Dec 17, 2016

Securing apps via obfuscating is a good start but it doesn't hold any good hacker to do his/her thing. (If there's an issue).

This is something Express does all the times.

What they have a way to turn it off. I'll think about something like that.

@rauchg
Copy link
Member

rauchg commented Dec 17, 2016

All major web servers do this.
It allows us to in the future actually responsibly disclose security problems!
That said, @arunoda it'd be helpful to introduce a config flag poweredByHeader: false to turn it off

@arunoda
Copy link
Contributor Author

arunoda commented Dec 17, 2016

@rauchg That's a good suggestion.
I'll hold this PR and do it once we've land #222

@arunoda
Copy link
Contributor Author

arunoda commented Dec 17, 2016

@rauchg check now.
We could disable setting the header.
Additionally, we could access the config via app.config if the user uses #310

@coveralls
Copy link

coveralls commented Dec 17, 2016

Coverage Status

Coverage decreased (-0.06%) to 57.422% when pulling ce5605c on arunoda:x-powered-by into 5ab7463 on zeit:master.

@rauchg
Copy link
Member

rauchg commented Dec 17, 2016

And a test would be nice to add as well!

@arunoda
Copy link
Contributor Author

arunoda commented Dec 18, 2016

@rauchg added some test cases.

@coveralls
Copy link

coveralls commented Dec 18, 2016

Coverage Status

Coverage increased (+3.2%) to 60.635% when pulling e2cb912 on arunoda:x-powered-by into 5ab7463 on zeit:master.

@rauchg rauchg merged commit 26c485a into vercel:master Dec 19, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants