-
-
Notifications
You must be signed in to change notification settings - Fork 57
Initial support #1
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
Changes from 34 commits
999be72
47c6564
6a4ab9b
1ed3bff
1b81ee0
1e3307c
7e7fbf6
43d4c48
bb43a07
656318b
40c2973
a246407
77360d6
6f5815b
5240539
8b4a078
14dac4e
8c3c90e
65261ee
7e44bc2
1e480a9
9127c84
b360068
4d11665
e129154
91a13dc
ef384ae
bcfa698
8a2bd7b
f41f3e4
9c6f180
7d8bdc8
eacce8f
7012db5
b887c36
ca2ea3b
577e633
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
vendor/ | ||
composer.lock | ||
phpunit.xml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
language: php | ||
|
||
sudo: false | ||
|
||
matrix: | ||
include: | ||
- php: 5.3 | ||
- php: 5.4 | ||
- php: 5.5 | ||
- php: 5.6 | ||
- php: 5.3 | ||
env: deps=low | ||
- php: 5.6 | ||
env: deps=high | ||
- php: nightly | ||
- php: hhvm | ||
allow_failures: | ||
- php: nightly | ||
- php: hhvm | ||
fast_finish: true | ||
|
||
env: | ||
global: | ||
- deps=no | ||
- SYMFONY_DEPRECATIONS_HELPER=weak | ||
|
||
before_install: | ||
- composer self-update | ||
- if [[ "$TRAVIS_PHP_VERSION" != "nightly" ]] && [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then phpenv config-rm xdebug.ini; fi; | ||
- if [[ "$TRAVIS_PHP_VERSION" != "nightly" ]] && [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]] && [ $(php -r "echo PHP_MINOR_VERSION;") -le 4 ]; then echo "extension = apc.so" >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini; fi; | ||
- if [[ "$TRAVIS_PHP_VERSION" != "nightly" ]] && [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]]; then php -i; fi; | ||
# Set the COMPOSER_ROOT_VERSION to the right version according to the branch being built | ||
- if [ "$TRAVIS_BRANCH" = "master" ]; then export COMPOSER_ROOT_VERSION=dev-master; else export COMPOSER_ROOT_VERSION="$TRAVIS_BRANCH".x-dev; fi; | ||
|
||
install: | ||
- if [[ "$TRAVIS_PHP_VERSION" != "5.3" ]] && [[ "$TRAVIS_PHP_VERSION" != "5.4" ]]; then composer require --no-update zendframework/zend-diactoros; fi; | ||
- if [ "$deps" = "no" ]; then export SYMFONY_DEPRECATIONS_HELPER=strict; fi; | ||
- if [ "$deps" = "no" ]; then composer --prefer-source install; fi; | ||
- if [ "$deps" = "high" ]; then composer --prefer-source update; fi; | ||
- if [ "$deps" = "low" ]; then composer --prefer-source --prefer-lowest --prefer-stable update; fi; | ||
|
||
script: | ||
- phpunit |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,159 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <[email protected]> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Bridge\PsrHttpMessage\Factory; | ||
|
||
use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; | ||
use Symfony\Component\HttpFoundation\File\UploadedFile; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
use Symfony\Component\HttpFoundation\StreamedResponse; | ||
use Zend\Diactoros\Response as DiactorosResponse; | ||
use Zend\Diactoros\ServerRequest; | ||
use Zend\Diactoros\ServerRequestFactory as DiactorosRequestFactory; | ||
use Zend\Diactoros\Stream as DiactorosStream; | ||
use Zend\Diactoros\UploadedFile as DiactorosUploadedFile; | ||
|
||
/** | ||
* Builds Psr\HttpMessage instances using the Zend Diactoros implementation. | ||
* | ||
* @author Kévin Dunglas <[email protected]> | ||
*/ | ||
class DiactorosFactory implements HttpMessageFactoryInterface | ||
{ | ||
public function __construct() | ||
{ | ||
if (!class_exists('Zend\Diactoros\ServerRequestFactory')) { | ||
throw new \RuntimeException('Zend Diactoros must be installed to use the DiactorosFactory.'); | ||
} | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createRequest(Request $symfonyRequest) | ||
{ | ||
$server = DiactorosRequestFactory::normalizeServer($symfonyRequest->server->all()); | ||
$headers = $symfonyRequest->headers->all(); | ||
|
||
try { | ||
$body = new DiactorosStream($symfonyRequest->getContent(true)); | ||
} catch (\LogicException $e) { | ||
$body = new DiactorosStream('php://temp', 'wb+'); | ||
$body->write($symfonyRequest->getContent()); | ||
} | ||
|
||
$request = new ServerRequest( | ||
$server, | ||
DiactorosRequestFactory::normalizeFiles($this->getFiles($symfonyRequest->files->all())), | ||
DiactorosRequestFactory::marshalUriFromServer($server, $headers), | ||
$symfonyRequest->getMethod(), | ||
$body, | ||
$headers | ||
); | ||
|
||
$request = $request | ||
->withCookieParams($symfonyRequest->cookies->all()) | ||
->withQueryParams($symfonyRequest->query->all()) | ||
->withParsedBody($symfonyRequest->request->all()) | ||
; | ||
|
||
foreach ($symfonyRequest->attributes->all() as $key => $value) { | ||
$request = $request->withAttribute($key, $value); | ||
} | ||
|
||
return $request; | ||
} | ||
|
||
/** | ||
* Converts Symfony uploaded files array to the PSR one. | ||
* | ||
* @param array $uploadedFiles | ||
* | ||
* @return array | ||
*/ | ||
private function getFiles(array $uploadedFiles) | ||
{ | ||
$files = array(); | ||
|
||
foreach ($uploadedFiles as $key => $value) { | ||
if ($value instanceof UploadedFile) { | ||
$files[$key] = $this->createUploadedFile($value); | ||
} else { | ||
$files[$key] = $this->getFiles($value); | ||
} | ||
} | ||
|
||
return $files; | ||
} | ||
|
||
/** | ||
* Creates a PSR-7 UploadedFile instance from a Symfony one. | ||
* | ||
* @param UploadedFile $symfonyUploadedFile | ||
* | ||
* @return UploadedFileInterface | ||
*/ | ||
private function createUploadedFile(UploadedFile $symfonyUploadedFile) | ||
{ | ||
return new DiactorosUploadedFile( | ||
$symfonyUploadedFile->getRealPath(), | ||
$symfonyUploadedFile->getSize(), | ||
$symfonyUploadedFile->getError(), | ||
$symfonyUploadedFile->getClientOriginalName(), | ||
$symfonyUploadedFile->getClientMimeType() | ||
); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function createResponse(Response $symfonyResponse) | ||
{ | ||
$stream = new DiactorosStream('php://temp', 'wb+'); | ||
if ($symfonyResponse instanceof StreamedResponse) { | ||
ob_start(function ($buffer) use ($stream) { | ||
$stream->write($buffer); | ||
|
||
return false; | ||
}); | ||
|
||
$symfonyResponse->sendContent(); | ||
ob_end_clean(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means that the streamed content will in fact be read into memory, correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because I use |
||
} else { | ||
$stream->write($symfonyResponse->getContent()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll take a look. |
||
} | ||
|
||
$headers = $symfonyResponse->headers->all(); | ||
|
||
$cookies = $symfonyResponse->headers->getCookies(); | ||
if (!empty($cookies)) { | ||
$headers['Set-Cookie'] = array(); | ||
|
||
foreach ($cookies as $coookie) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why 3 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo |
||
$headers['Set-Cookie'][] = $coookie->__toString(); | ||
} | ||
} | ||
|
||
$response = new DiactorosResponse( | ||
$stream, | ||
$symfonyResponse->getStatusCode(), | ||
$headers | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The conversion is incomplete here:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok cookies are not added as headers in Symfony Response. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is not always hardcoded: https://github.com/zendframework/zend-diactoros/blob/be067c409377671b8268236443d993374e1987fa/src/Response.php#L160 (which is very similar to what we do in HttpFoundation btw) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missed it but still not usable because there is not getter for the status text in the Symfony response. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, if we don't convert the status text, it is not a big issue. I don't think it is the most used feature in HttpFoundation (I'm not even sure it is documented, and setting the status code in the constructor does not allow to set the status text). |
||
|
||
$protocolVersion = $symfonyResponse->getProtocolVersion(); | ||
if ('1.1' !== $protocolVersion) { | ||
$response = $response->withProtocolVersion($protocolVersion); | ||
} | ||
|
||
return $response; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not reusing
$symfonyRequest->getRequestUri()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because Symfony sometimes generates bad URIs. For instance with the request I build in the test, the generated URL is
http://:/
(because some $_SERVER parameters are missing).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like @stof's general idea - we should use as much information from Symfony's Request as possible, since it's already calculated there and I expect information to be the same. For example, what if the scheme is
http
in Symfony's Request, but due to somex-forwarded-proto
differences (perhaps you don't have something a proxy trusted in Symfony's Request), Diactoros reads the untrustedx-forwarded-proto
and treats it ashttps
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not. So I'll fix the test with the correct $_SEVER keys but if we got some complaints about strange bugs here we should fix HttpFoundation directly.