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

Already on GitHub? Sign in to your account

Don't escape text/plain as HTML in the default 404 handler #873

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

johan commented Aug 23, 2013

Before:

Cannot GET http://localhost/?a=b&c=d

After:

Cannot GET http://localhost/?a=b&c=d
Don't escape text/plain as HTML in the default 404 handler
Before: `Cannot GET http://localhost/?a=b&c=d`
After:  `Cannot GET http://localhost/?a=b&c=d`
Contributor

JacksonTian commented Aug 24, 2013

test case will great.

Contributor

johan commented Aug 24, 2013

Good point – addressed.

Member

tj commented Aug 24, 2013

it's escaped because some clients will misinterpret as html and leave you open for xss

@tj tj closed this Aug 24, 2013

Contributor

johan commented Aug 24, 2013

Would you then accept a pull request to change it to send content-type: text/html and escaping it correctly? Optionally wrapped in a <pre> tag, if monospace is the end goal?

The current behaviour makes apps hard to debug, as it hints about a misquoting somewhere in your app, which is really just happening in connect and is a red herring.

Contributor

johan commented Aug 24, 2013

Opposite fix in #875.

Member

tj commented Aug 28, 2013

looks good, I'd remove the

 so tools like curl don't get less readable output

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment