Remove useless code and fix double headers overwriting #48
Conversation
dzubchik
changed the title
| @@ -10,6 +10,7 @@ | |||
| use React\Http\Request as ReactRequest; | |||
| use Symfony\Component\HttpFoundation\Cookie; | |||
| use Symfony\Component\HttpFoundation\Request as SymfonyRequest; | |||
| use Symfony\Component\HttpFoundation\Request; | |||
There was a problem hiding this comment.
You have the SymfonyRequest already?
| $_SERVER['PHP_AUTH_USER'] = ''; | ||
| $_SERVER['PHP_AUTH_PW'] = ''; | ||
| $_SERVER['AUTH_TYPE'] = ''; | ||
| unset($headers['PHP_AUTH_USER'], $headers['PHP_AUTH_PW'], $headers['AUTH_TYPE']); |
There was a problem hiding this comment.
Are these guaranteed to be set or will array access throw a warning?
There was a problem hiding this comment.
| $_SERVER['AUTH_TYPE'] = $type; | ||
| $headers['PHP_AUTH_USER'] = isset($credentialsParts[0]) ? $credentialsParts[0] : ''; | ||
| $headers['PHP_AUTH_PW'] = isset($credentialsParts[1]) ? $credentialsParts[1] : ''; | ||
| $headers['AUTH_TYPE'] = $type; | ||
| } |
There was a problem hiding this comment.
This would be an addon: what about Bearer authentication? It is today in symfony only available from the apache headers. Any idea how to get those across?
There was a problem hiding this comment.
Maybe we can use ServerBag to handle authorization headers?
|
@andig, @marcj what do you think about implementing better headers parser in react/http? Or we should reset $_SERVER values in each request? |
|
@marcj, @andig , I have a little updated this PR after seeing this commit. Now it works correctly with headers and auth headers parsing is moving to symfony request. I removed line with headers merging, because I could not find any example when symfony and react headers could be different. If you insist, I will return that line. But all my previous code should be removed from this adapter. |
| @@ -177,10 +157,10 @@ protected function mapRequest(ReactRequest $reactRequest) | |||
| $class = '\Symfony\Component\HttpFoundation\Request'; | |||
| } | |||
|
|
|||
| $syRequest = new $class($query, $post, $attributes = [], $cookies, $files, $_SERVER, $reactRequest->getBody()); | |||
| /** @var SymfonyRequest $syRequest */ | |||
| $syRequest = new $class($query, $post, $attributes = [], $_COOKIE, $files, $_SERVER, $reactRequest->getBody()); | |||
There was a problem hiding this comment.
so you user and password in the symfony request with this change?
There was a problem hiding this comment.
Would it be possiblr to create a new, clean pr and describe the problem it tries to solve? Must say that I'm totally lost by now.
dzubchik
changed the title
|
First, I've created PR #46 because headers from react request rewrite all headers from symfony request. Then, I've opened the first version of this PR, because of bug when during application runtime $ _SERVER is just increasing overtime, so the headers are not consistant with current request. But, @marcj had fixed this in php-pm/php-pm@9b63b99, so I've rewrote my PR, only to remove my old code with auth headers, because now Symfony request handle all of this correctly and remove $ cookie, because it is useless. |
|
@dzubchik thanks, good job! |
No description provided.