From 889f2dffa3e8badcd0791968228dbd9c4b664877 Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Fri, 21 Jul 2017 11:00:22 +0200 Subject: [PATCH 1/9] Add Content-type plugin to set content-type header automatically (for now just JSON and XML) --- spec/Plugin/ContentTypePluginSpec.php | 59 +++++++++++++++++++++++++ src/Plugin/ContentTypePlugin.php | 62 +++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 spec/Plugin/ContentTypePluginSpec.php create mode 100644 src/Plugin/ContentTypePlugin.php diff --git a/spec/Plugin/ContentTypePluginSpec.php b/spec/Plugin/ContentTypePluginSpec.php new file mode 100644 index 0000000..4074e00 --- /dev/null +++ b/spec/Plugin/ContentTypePluginSpec.php @@ -0,0 +1,59 @@ +shouldHaveType('Http\Client\Common\Plugin\ContentTypePlugin'); + } + + function it_is_a_plugin() + { + $this->shouldImplement('Http\Client\Common\Plugin'); + } + + function it_adds_json_content_type_header(RequestInterface $request) + { + $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); + $request->getBody()->shouldBeCalled()->willReturn(json_encode(['foo' => 'bar'])); + $request->withHeader('Content-Type', 'application/json')->shouldBeCalled()->willReturn($request); + + $this->handleRequest($request, function () {}, function () {}); + } + + function it_adds_xml_content_type_header(RequestInterface $request) + { + $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); + $request->getBody()->shouldBeCalled()->willReturn('bar'); + $request->withHeader('Content-Type', 'application/xml')->shouldBeCalled()->willReturn($request); + + $this->handleRequest($request, function () {}, function () {}); + } + + function it_does_not_set_content_type_header(RequestInterface $request) + { + $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); + $request->getBody()->shouldBeCalled()->willReturn('foo'); + $request->withHeader('Content-Type', null)->shouldNotBeCalled(); + + $this->handleRequest($request, function () {}, function () {}); + } + + function it_does_not_set_content_type_header_if_already_one(RequestInterface $request) + { + $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(true); + $request->getBody()->shouldNotBeCalled()->willReturn('foo'); + $request->withHeader('Content-Type', null)->shouldNotBeCalled(); + + $this->handleRequest($request, function () {}, function () {}); + } + +} diff --git a/src/Plugin/ContentTypePlugin.php b/src/Plugin/ContentTypePlugin.php new file mode 100644 index 0000000..b2f9e70 --- /dev/null +++ b/src/Plugin/ContentTypePlugin.php @@ -0,0 +1,62 @@ + + */ +final class ContentTypePlugin implements Plugin +{ + /** + * {@inheritdoc} + */ + public function handleRequest(RequestInterface $request, callable $next, callable $first) + { + if (!$request->hasHeader('Content-Type')) { + $stream = $request->getBody(); + + if ($this->isJson($stream)) { + $request = $request->withHeader('Content-Type', 'application/json'); + + return $next($request); + } + + if ($this->isXml($stream)) { + $request = $request->withHeader('Content-Type', 'application/xml'); + + return $next($request); + } + } + + return $next($request); + } + + /** + * @param $stream + * + * @return bool + */ + private function isJson($stream) + { + json_decode($stream); + + return json_last_error() == JSON_ERROR_NONE; + } + + /** + * @param $stream + * + * @return \SimpleXMLElement|false + */ + private function isXml($stream) + { + libxml_use_internal_errors(true); + + return simplexml_load_string($stream); + } +} From 5d43a8613156e071271dc81edd88e724ff74ddf1 Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Fri, 21 Jul 2017 15:40:34 +0200 Subject: [PATCH 2/9] Use guzzle/psr7 implementation for unit tests --- composer.json | 3 ++- spec/Plugin/ContentTypePluginSpec.php | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 69262e6..35db76d 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,8 @@ }, "require-dev": { "phpspec/phpspec": "^2.4", - "henrikbjorn/phpspec-code-coverage" : "^1.0" + "henrikbjorn/phpspec-code-coverage" : "^1.0", + "guzzlehttp/psr7": "^1.4" }, "suggest": { "php-http/logger-plugin": "PSR-3 Logger plugin", diff --git a/spec/Plugin/ContentTypePluginSpec.php b/spec/Plugin/ContentTypePluginSpec.php index 4074e00..f1bc6c2 100644 --- a/spec/Plugin/ContentTypePluginSpec.php +++ b/spec/Plugin/ContentTypePluginSpec.php @@ -23,7 +23,7 @@ function it_is_a_plugin() function it_adds_json_content_type_header(RequestInterface $request) { $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); - $request->getBody()->shouldBeCalled()->willReturn(json_encode(['foo' => 'bar'])); + $request->getBody()->shouldBeCalled()->willReturn(\GuzzleHttp\Psr7\stream_for(json_encode(['foo' => 'bar']))); $request->withHeader('Content-Type', 'application/json')->shouldBeCalled()->willReturn($request); $this->handleRequest($request, function () {}, function () {}); @@ -32,7 +32,7 @@ function it_adds_json_content_type_header(RequestInterface $request) function it_adds_xml_content_type_header(RequestInterface $request) { $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); - $request->getBody()->shouldBeCalled()->willReturn('bar'); + $request->getBody()->shouldBeCalled()->willReturn(\GuzzleHttp\Psr7\stream_for('bar')); $request->withHeader('Content-Type', 'application/xml')->shouldBeCalled()->willReturn($request); $this->handleRequest($request, function () {}, function () {}); @@ -41,7 +41,7 @@ function it_adds_xml_content_type_header(RequestInterface $request) function it_does_not_set_content_type_header(RequestInterface $request) { $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); - $request->getBody()->shouldBeCalled()->willReturn('foo'); + $request->getBody()->shouldBeCalled()->willReturn(\GuzzleHttp\Psr7\stream_for('foo')); $request->withHeader('Content-Type', null)->shouldNotBeCalled(); $this->handleRequest($request, function () {}, function () {}); @@ -50,7 +50,7 @@ function it_does_not_set_content_type_header(RequestInterface $request) function it_does_not_set_content_type_header_if_already_one(RequestInterface $request) { $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(true); - $request->getBody()->shouldNotBeCalled()->willReturn('foo'); + $request->getBody()->shouldNotBeCalled()->willReturn(\GuzzleHttp\Psr7\stream_for('foo')); $request->withHeader('Content-Type', null)->shouldNotBeCalled(); $this->handleRequest($request, function () {}, function () {}); From 9ef69cf524869a9684924b4068baf7f9ba3545da Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Fri, 21 Jul 2017 15:42:43 +0200 Subject: [PATCH 3/9] Return early if body size is 0 or unknown --- spec/Plugin/ContentTypePluginSpec.php | 9 +++++++++ src/Plugin/ContentTypePlugin.php | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/spec/Plugin/ContentTypePluginSpec.php b/spec/Plugin/ContentTypePluginSpec.php index f1bc6c2..a1de4f8 100644 --- a/spec/Plugin/ContentTypePluginSpec.php +++ b/spec/Plugin/ContentTypePluginSpec.php @@ -56,4 +56,13 @@ function it_does_not_set_content_type_header_if_already_one(RequestInterface $re $this->handleRequest($request, function () {}, function () {}); } + function it_does_not_set_content_type_header_if_size_0_or_unknown(RequestInterface $request) + { + $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); + $request->getBody()->shouldBeCalled()->willReturn(\GuzzleHttp\Psr7\stream_for()); + $request->withHeader('Content-Type', null)->shouldNotBeCalled(); + + $this->handleRequest($request, function () {}, function () {}); + } + } diff --git a/src/Plugin/ContentTypePlugin.php b/src/Plugin/ContentTypePlugin.php index b2f9e70..e1573a5 100644 --- a/src/Plugin/ContentTypePlugin.php +++ b/src/Plugin/ContentTypePlugin.php @@ -19,6 +19,11 @@ public function handleRequest(RequestInterface $request, callable $next, callabl { if (!$request->hasHeader('Content-Type')) { $stream = $request->getBody(); + $streamSize = $stream->getSize(); + + if (0 == $streamSize) { + return $next($request); + } if ($this->isJson($stream)) { $request = $request->withHeader('Content-Type', 'application/json'); From 2d1a8850b4a0926f52ce6a0e7e78d2dc2728c15c Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Fri, 21 Jul 2017 15:47:59 +0200 Subject: [PATCH 4/9] Reset the previous value at the end --- src/Plugin/ContentTypePlugin.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Plugin/ContentTypePlugin.php b/src/Plugin/ContentTypePlugin.php index e1573a5..84f77b7 100644 --- a/src/Plugin/ContentTypePlugin.php +++ b/src/Plugin/ContentTypePlugin.php @@ -60,8 +60,10 @@ private function isJson($stream) */ private function isXml($stream) { - libxml_use_internal_errors(true); + $previousValue = libxml_use_internal_errors(true); + $isXml = simplexml_load_string($stream); + libxml_use_internal_errors($previousValue); - return simplexml_load_string($stream); + return $isXml; } } From bc7b746ef9b3c3e68c892b9cf67c8922d52185a1 Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Fri, 21 Jul 2017 16:15:55 +0200 Subject: [PATCH 5/9] Rewind stream before each content test --- src/Plugin/ContentTypePlugin.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Plugin/ContentTypePlugin.php b/src/Plugin/ContentTypePlugin.php index 84f77b7..b46c70d 100644 --- a/src/Plugin/ContentTypePlugin.php +++ b/src/Plugin/ContentTypePlugin.php @@ -4,6 +4,7 @@ use Http\Client\Common\Plugin; use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\StreamInterface; /** * Allow to set the correct content type header on the request automatically only if it is not set . @@ -42,26 +43,30 @@ public function handleRequest(RequestInterface $request, callable $next, callabl } /** - * @param $stream + * @param $stream StreamInterface * * @return bool */ private function isJson($stream) { - json_decode($stream); + $stream->rewind(); + + json_decode($stream->getContents()); return json_last_error() == JSON_ERROR_NONE; } /** - * @param $stream + * @param $stream StreamInterface * * @return \SimpleXMLElement|false */ private function isXml($stream) { + $stream->rewind(); + $previousValue = libxml_use_internal_errors(true); - $isXml = simplexml_load_string($stream); + $isXml = simplexml_load_string($stream->getContents()); libxml_use_internal_errors($previousValue); return $isXml; From 77d63c4f5921de95e6eda88a0e585dd66cddd118 Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Fri, 21 Jul 2017 17:15:32 +0200 Subject: [PATCH 6/9] Add options to skip detection for too large stream --- spec/Plugin/ContentTypePluginSpec.php | 41 +++++++++++++++++++++++ src/Plugin/ContentTypePlugin.php | 47 ++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/spec/Plugin/ContentTypePluginSpec.php b/spec/Plugin/ContentTypePluginSpec.php index a1de4f8..3df7d87 100644 --- a/spec/Plugin/ContentTypePluginSpec.php +++ b/spec/Plugin/ContentTypePluginSpec.php @@ -65,4 +65,45 @@ function it_does_not_set_content_type_header_if_size_0_or_unknown(RequestInterfa $this->handleRequest($request, function () {}, function () {}); } + function it_adds_xml_content_type_header_if_size_limit_is_not_reached_using_default_value(RequestInterface $request) + { + $this->beConstructedWith([ + 'skip_detection' => true + ]); + + $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); + $request->getBody()->shouldBeCalled()->willReturn(\GuzzleHttp\Psr7\stream_for('bar')); + $request->withHeader('Content-Type', 'application/xml')->shouldBeCalled()->willReturn($request); + + $this->handleRequest($request, function () {}, function () {}); + } + + function it_adds_xml_content_type_header_if_size_limit_is_not_reached(RequestInterface $request) + { + $this->beConstructedWith([ + 'skip_detection' => true, + 'size_limit' => 32000000 + ]); + + $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); + $request->getBody()->shouldBeCalled()->willReturn(\GuzzleHttp\Psr7\stream_for('bar')); + $request->withHeader('Content-Type', 'application/xml')->shouldBeCalled()->willReturn($request); + + $this->handleRequest($request, function () {}, function () {}); + } + + function it_does_not_set_content_type_header_if_size_limit_is_reached(RequestInterface $request) + { + $this->beConstructedWith([ + 'skip_detection' => true, + 'size_limit' => 8 + ]); + + $request->hasHeader('Content-Type')->shouldBeCalled()->willReturn(false); + $request->getBody()->shouldBeCalled()->willReturn(\GuzzleHttp\Psr7\stream_for('bar')); + $request->withHeader('Content-Type', null)->shouldNotBeCalled(); + + $this->handleRequest($request, function () {}, function () {}); + } + } diff --git a/src/Plugin/ContentTypePlugin.php b/src/Plugin/ContentTypePlugin.php index b46c70d..28276c7 100644 --- a/src/Plugin/ContentTypePlugin.php +++ b/src/Plugin/ContentTypePlugin.php @@ -5,14 +5,55 @@ use Http\Client\Common\Plugin; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\StreamInterface; +use Symfony\Component\OptionsResolver\OptionsResolver; /** - * Allow to set the correct content type header on the request automatically only if it is not set . + * Allow to set the correct content type header on the request automatically only if it is not set. * * @author Karim Pinchon */ final class ContentTypePlugin implements Plugin { + /** + * Allow to disable the content type detection when stream is too large (as it can consume a lot of resource). + * + * @var bool + * + * true skip the content type detection + * false detect the content type (default value) + */ + protected $skipDetection; + + /** + * Determine the size stream limit for which the detection as to be skipped (default to 16Mb). + * + * @var int + */ + protected $sizeLimit; + + /** + * @param array $config { + * + * @var bool $skip_detection True skip detection if stream size is bigger than $size_limit. + * @var int $size_limit size stream limit for which the detection as to be skipped. + * } + */ + public function __construct(array $config = []) + { + $resolver = new OptionsResolver(); + $resolver->setDefaults([ + 'skip_detection' => false, + 'size_limit' => 16000000, + ]); + $resolver->setAllowedTypes('skip_detection', 'bool'); + $resolver->setAllowedTypes('size_limit', 'int'); + + $options = $resolver->resolve($config); + + $this->skipDetection = $options['skip_detection']; + $this->sizeLimit = $options['size_limit']; + } + /** * {@inheritdoc} */ @@ -26,6 +67,10 @@ public function handleRequest(RequestInterface $request, callable $next, callabl return $next($request); } + if ($this->skipDetection && $streamSize >= $this->sizeLimit) { + return $next($request); + } + if ($this->isJson($stream)) { $request = $request->withHeader('Content-Type', 'application/json'); From 61629417da919cee02958b23b29bb2d7599827a5 Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Fri, 21 Jul 2017 17:18:47 +0200 Subject: [PATCH 7/9] Skip detection if stream is not seekable --- src/Plugin/ContentTypePlugin.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Plugin/ContentTypePlugin.php b/src/Plugin/ContentTypePlugin.php index 28276c7..38f0593 100644 --- a/src/Plugin/ContentTypePlugin.php +++ b/src/Plugin/ContentTypePlugin.php @@ -63,6 +63,10 @@ public function handleRequest(RequestInterface $request, callable $next, callabl $stream = $request->getBody(); $streamSize = $stream->getSize(); + if (!$stream->isSeekable()) { + return $next($request); + } + if (0 == $streamSize) { return $next($request); } From e6ba6f167663f23d6b2fd3ab5901a9f2cf24e40b Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Tue, 25 Jul 2017 09:20:27 +0200 Subject: [PATCH 8/9] Add null comparison --- src/Plugin/ContentTypePlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Plugin/ContentTypePlugin.php b/src/Plugin/ContentTypePlugin.php index 38f0593..271c080 100644 --- a/src/Plugin/ContentTypePlugin.php +++ b/src/Plugin/ContentTypePlugin.php @@ -67,7 +67,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl return $next($request); } - if (0 == $streamSize) { + if (null === $streamSize || 0 === $streamSize) { return $next($request); } From 93c6bf27dd4105cae0ec25c0381e7bb8e5c90d3f Mon Sep 17 00:00:00 2001 From: Karim PINCHON Date: Tue, 25 Jul 2017 10:07:15 +0200 Subject: [PATCH 9/9] Check if stream size is null only when skipDetection is true --- src/Plugin/ContentTypePlugin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Plugin/ContentTypePlugin.php b/src/Plugin/ContentTypePlugin.php index 271c080..038b3e4 100644 --- a/src/Plugin/ContentTypePlugin.php +++ b/src/Plugin/ContentTypePlugin.php @@ -67,11 +67,11 @@ public function handleRequest(RequestInterface $request, callable $next, callabl return $next($request); } - if (null === $streamSize || 0 === $streamSize) { + if (0 === $streamSize) { return $next($request); } - if ($this->skipDetection && $streamSize >= $this->sizeLimit) { + if ($this->skipDetection && (null === $streamSize || $streamSize >= $this->sizeLimit)) { return $next($request); }