-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add REQUEST_PATH on parse error message #1831
Conversation
f15331c
to
9d44dac
Compare
Works for me. Test? |
Added in ea555ef |
Would it be possible to log the full url? Instead of just the path |
Awesome, thanks for adding a test ❤️ |
If you mean with the full url adding the query string in this case it wouldn't be possible since we cannot parse it, the QUERY_STRING is going to be empty. For the IP its going to be present when puma is behind an http roxy forwarding the env x-forwarded-for or remote-addr cf https://github.com/spk/puma/blob/ea555ef9d617782c03ca0554ca8069d92f109838/lib/puma/events.rb#L97 I hope I answer your question and of course I let Nate correct me if I'm wrong ! |
I appreciate the effort you put in to the test here, but I'm trying to avoid adding more integration tests like this unless absolutely necessary. I'd prefer a simpler test like the one here., or an even simpler one that simply calls parse_error and checks stdout. |
I've looked up I don't think its possible to check the stderr log here, we already have a test for max params and this raise directly... Maybe I'm wrong thanks for your help! |
@spk Since the method you're changing is actually in Puma::Events, you'll need to start a new Puma::Server with a custom IO object (don't use global stderr/stdout), and then you can cause a parser error and check the IO object. Maybe the tests for Server or Events would be a better inspiration. I would put this test in with the event tests because that's where this method actually lives. |
``` 2019-07-01 21:31:08 +0200: HTTP parse error, malformed request (127.0.0.1/plop): #<Puma::HttpParserError: HTTP element QUERY_STRING is longer than the (1024 * 10) allowed length (was 20481)> ``` ref puma#1768
ea555ef
to
55334b2
Compare
Thanks Nate for your help hope its ok with this spk@55334b2 |
Request get
/plop
and query string bigger than 1024*10ref #1768