[0.3] Replace guzzle/http with custom implementation #131
Conversation
| private function formatHead($status, array $headers) | ||
| { | ||
| $text = ResponseCodes::$statusTexts[$status]; | ||
| $data = "HTTP/1.1 $status $text\r\n"; |
There was a problem hiding this comment.
you should verify that $status and $text cannot contain "\r\n" or "\n" to protect against http-header injections
There was a problem hiding this comment.
These are from ResponseCodes, which is a trusted source.
There was a problem hiding this comment.
$status is not enforced to be a number, therefore could also contain "\r\n" and finally used for a injection
There was a problem hiding this comment.
I like your attention to detail, fixed.
|
Any particular reason why no use i.e. Buzz ? |
|
It has a hard dependency on ext-curl. React aims to work without third party exts. Also, ext-curl does not allow for file descriptor polling, which means there is no sane way to integrate it into an event loop. EDIT: To clarify, ext-curl does not expose a file descriptor to poll on. |
|
I understand that Guzzle has hard deps on curl ext. but Buzz don't have such. I know it's not perfect but... =) |
|
Yeah, you see that |
|
@staabm I fixed the things you mentioned, thanks! |
We have decided to no longer depend on guzzle/http. We will still be using guzzle/parser.
The reason is that guzzle/http has added a dependency on ext-curl, and I'd rather isolate react from such unexpected changes in the future. Also, we are only using a very narrow subset of what guzzle/http provides, which drags in quite a few unnecessary dependencies.
This change lifts deps on:
I have written minimal implementations of request and response building. Please test them extensively.