From 6450c425c22fd6295d830714241641fe0223474c Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 8 May 2017 11:05:16 -0500 Subject: [PATCH 1/2] Do not compare + segments unless wildcards in either accept or priority Fixes #93. If there are no wildcard segments in either the subtype or + segment of either the Accept header or priority, do not attempt to compare them. Adds more tests for #92 as well, using `*` subtypes in both the Accept header and priorities, to validate that different combinations work as expected. --- src/Negotiation/Negotiator.php | 10 ++++++++-- tests/Negotiation/Tests/NegotiatorTest.php | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Negotiation/Negotiator.php b/src/Negotiation/Negotiator.php index 7dbd724..41bfeac 100644 --- a/src/Negotiation/Negotiator.php +++ b/src/Negotiation/Negotiator.php @@ -49,11 +49,17 @@ protected function match(AcceptHeader $accept, AcceptHeader $priority, $index) list($acceptSub, $acceptPlus) = $this->splitSubPart($acceptSub); list($prioritySub, $priorityPlus) = $this->splitSubPart($prioritySub); + // If no wildcards in either the subtype or + segment, do nothing. + if (! ($acceptBase === '*' || $baseEqual) + || ! ($acceptSub === '*' || $prioritySub === '*' || $acceptPlus === '*' || $priorityPlus === '*')) { + return null; + } + $subEqual = !strcasecmp($acceptSub, $prioritySub); $plusEqual = !strcasecmp($acceptPlus, $priorityPlus); - if (($acceptBase === '*' || $baseEqual) - && ($acceptSub === '*' || $subEqual || $acceptPlus === '*' || $plusEqual) + if (($acceptSub === '*' || $prioritySub === '*' || $subEqual) + && ($acceptPlus === '*' || $priorityPlus === '*' || $plusEqual) && count($intersection) === count($accept->getParameters()) ) { $score = 100 * $baseEqual + 10 * $subEqual + $plusEqual + count($intersection); diff --git a/tests/Negotiation/Tests/NegotiatorTest.php b/tests/Negotiation/Tests/NegotiatorTest.php index 79f585b..fe3ff76 100644 --- a/tests/Negotiation/Tests/NegotiatorTest.php +++ b/tests/Negotiation/Tests/NegotiatorTest.php @@ -96,7 +96,11 @@ public static function dataProviderForTestGetBest() array($rfcHeader, array('text/html;q=0.4', 'text/plain'), array('text/plain', array())), # Wildcard "plus" parts (e.g., application/vnd.api+json) array('application/vnd.api+json', array('application/json', 'application/*+json'), array('application/*+json', array())), + array('application/json;q=0.7, application/*+json;q=0.7', array('application/hal+json', 'application/problem+json'), array('application/hal+json', array())), + array('application/json;q=0.7, application/problem+*;q=0.7', array('application/hal+xml', 'application/problem+xml'), array('application/problem+xml', array())), array($pearAcceptHeader, array('application/*+xml'), array('application/*+xml', array())), + # @see https://github.com/willdurand/Negotiation/issues/93 + array('application/hal+json', array('application/ld+json', 'application/hal+json', 'application/xml', 'text/xml', 'application/json', 'text/html'), array('application/hal+json', array())), ); } From 516caf0fbb7337e96a34d49a542be40f875ae5cd Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Tue, 9 May 2017 08:02:12 -0500 Subject: [PATCH 2/2] CS fixes - Move closing paren/open brace to following line - No space following negation operator --- src/Negotiation/Negotiator.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Negotiation/Negotiator.php b/src/Negotiation/Negotiator.php index 41bfeac..d083fa1 100644 --- a/src/Negotiation/Negotiator.php +++ b/src/Negotiation/Negotiator.php @@ -50,8 +50,9 @@ protected function match(AcceptHeader $accept, AcceptHeader $priority, $index) list($prioritySub, $priorityPlus) = $this->splitSubPart($prioritySub); // If no wildcards in either the subtype or + segment, do nothing. - if (! ($acceptBase === '*' || $baseEqual) - || ! ($acceptSub === '*' || $prioritySub === '*' || $acceptPlus === '*' || $priorityPlus === '*')) { + if (!($acceptBase === '*' || $baseEqual) + || !($acceptSub === '*' || $prioritySub === '*' || $acceptPlus === '*' || $priorityPlus === '*') + ) { return null; }