Fixed URI parsing #207 #209
Conversation
clue
left a comment
There was a problem hiding this comment.
Thanks for spotting this issue and filing this PR! 👍
I've successfully reproduced the issue you're seeing and would love to get a fix for this out! 
However, it looks like the code you've added may cause issues for absolute-form requests (https://tools.ietf.org/html/rfc7230#section-5.3.2), while this should really affect origin-form requests (https://tools.ietf.org/html/rfc7230#section-5.3.1) only. May I ask you to look into this? Thanks!
src/RequestHeaderParser.php
Outdated
|
|
||
| // make sure value contains valid host component (IP or hostname), but no fragment | ||
| if (!isset($parts['scheme'], $parts['host']) || $parts['scheme'] !== 'http' || isset($parts['fragment'])) { | ||
| if ((!isset($parts['scheme'], $parts['host']) || $parts['scheme'] !== 'http' || isset($parts['fragment'])) && isset($parts['scheme'])) { |
There was a problem hiding this comment.
This is now really hard to read and I'm not sure it does what it's supposed to do anymore. It looks like a request-target that starts with :// will now pass this validation?
As an alternative, may I suggest reverting this and updating the above check instead? Your issue can only really happen for origin-target requests, which means we can actually skip this whole block if the request-target starts with a /.
Does this make sense to you?
There was a problem hiding this comment.
Yes, this is very difficult to read; I'll update it.
clue
left a comment
There was a problem hiding this comment.
Thanks for spotting and the quick update!
Closes #207