Skip to content

Commit

Permalink
Updated PSR-2 Checks. Fixes #3 "Sanitizing coordinate values for geod…
Browse files Browse the repository at this point in the history
…ist makes invalid query." Thanks @sunthera
  • Loading branch information
Fabio Piro committed Feb 5, 2017
1 parent d3b0ea4 commit 7cd5c61
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 80 deletions.
75 changes: 44 additions & 31 deletions src/Minimalcode/Search/Criteria.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ class Criteria
*/
private function __construct($field, Node $root)
{
if(! \is_string($field)) {
if (!\is_string($field)) {
throw new InvalidArgumentException('Criteria\'s field must be a string type.');
}

if($field === '') {
if ($field === '') {
throw new InvalidArgumentException('Field name for criteria must not be null/empty.');
}

Expand Down Expand Up @@ -96,9 +96,9 @@ public static function where($field)
* @return $this
* @throws \InvalidArgumentException if field is not a string or a Criteria object
*/
public function andWhere($field)
public function andWhere($field)
{
if($field instanceof Criteria) {
if ($field instanceof Criteria) {
return $this->root->inject(Node::OPERATOR_AND, $field->root)->getMostRecentCriteria();
}

Expand All @@ -112,9 +112,9 @@ public function andWhere($field)
* @return $this
* @throws \InvalidArgumentException if field is not a string or a Criteria object
*/
public function orWhere($field)
public function orWhere($field)
{
if($field instanceof Criteria) {
if ($field instanceof Criteria) {
return $this->root->inject(Node::OPERATOR_OR, $field->root)->getMostRecentCriteria();
}

Expand All @@ -134,7 +134,8 @@ public function between($lowerBound, $upperBound, $includeLowerBound = true, $in
{
$lowerBound = ($lowerBound === null) ? '*' : $lowerBound;
$upperBound = ($upperBound === null) ? '*' : $upperBound;
$this->predicates[] = ($includeLowerBound ? '[' : '{') . $this->processValue($lowerBound) . ' TO ' . $this->processValue($upperBound) . ($includeUpperBound ? ']' : '}');
$this->predicates[] = ($includeLowerBound ? '[' : '{') . $this->processValue($lowerBound) .
' TO ' . $this->processValue($upperBound) . ($includeUpperBound ? ']' : '}');

return $this;
}
Expand Down Expand Up @@ -193,11 +194,11 @@ public function greaterThanEqual($lowerBound)
*/
public function is($value)
{
if($value === null) {
if ($value === null) {
return $this->isNull();
}

if(\is_array($value)) {
if (\is_array($value)) {
return $this->in($value);
}

Expand All @@ -215,7 +216,7 @@ public function is($value)
public function in(array $values)
{
foreach ($values as $value) {
if(\is_array($value)) {
if (\is_array($value)) {
$this->in($value);
} else {
$this->is($value);
Expand All @@ -240,7 +241,9 @@ public function in(array $values)
public function withinCircle($latitude, $longitude, $distance)
{
$this->assertPositiveFloat($distance);
$this->predicates[] = '{!geofilt pt=' . $this->processValue($latitude) . ',' . $this->processValue($longitude) . ' sfield=' . $this->field . ' d=' . $this->processFloat($distance) . '}';
$this->predicates[] = '{!geofilt pt=' . $this->processFloat($latitude) . ',' .
$this->processFloat($longitude) . ' sfield=' . $this->field . ' d=' . $this->processFloat($distance) . '}';

$this->isHideFieldName = true;

return $this;
Expand All @@ -259,7 +262,8 @@ public function withinCircle($latitude, $longitude, $distance)
*/
public function withinBox($startLatitude, $startLongitude, $endLatitude, $endLongitude)
{
$this->predicates[] = '[' . $this->processValue($startLatitude) . ',' . $this->processValue($startLongitude) . ' TO ' . $this->processValue($endLatitude) . ',' . $this->processValue($endLongitude) . ']';
$this->predicates[] = '[' . $this->processFloat($startLatitude) . ',' . $this->processFloat($startLongitude) .
' TO ' . $this->processFloat($endLatitude) . ',' . $this->processFloat($endLongitude) . ']';

return $this;
}
Expand All @@ -280,7 +284,9 @@ public function withinBox($startLatitude, $startLongitude, $endLatitude, $endLon
public function nearCircle($latitude, $longitude, $distance)
{
$this->assertPositiveFloat($distance);
$this->predicates[] = '{!bbox pt=' . $this->processValue($latitude) . ',' . $this->processValue($longitude) . ' sfield=' . $this->field . ' d=' . $this->processFloat($distance) . '}';
$this->predicates[] = '{!bbox pt=' . $this->processFloat($latitude) . ',' . $this->processFloat($longitude) .
' sfield=' . $this->field . ' d=' . $this->processFloat($distance) . '}';

$this->isHideFieldName = true;

return $this;
Expand Down Expand Up @@ -314,7 +320,8 @@ public function isNotNull()
*/
public function contains($value)
{
if(\is_array($value)) {
if (\is_array($value)) {
/** @noinspection ForeachSourceInspection */
foreach ($value as $item) {
$this->contains($item);
}
Expand All @@ -334,7 +341,8 @@ public function contains($value)
*/
public function startsWith($prefix)
{
if(\is_array($prefix)) {
if (\is_array($prefix)) {
/** @noinspection ForeachSourceInspection */
foreach ($prefix as $item) {
$this->startsWith($item);
}
Expand All @@ -355,7 +363,8 @@ public function startsWith($prefix)
*/
public function endsWith($postfix)
{
if(\is_array($postfix)) {
if (\is_array($postfix)) {
/** @noinspection ForeachSourceInspection */
foreach ($postfix as $item) {
$this->endsWith($item);
}
Expand Down Expand Up @@ -405,7 +414,8 @@ public function fuzzy($value, $levenshteinDistance = null)
throw new InvalidArgumentException('Levenshtein Distance has to be within its bounds (0.0 - 1.0).');
}

$this->predicates[] = $this->processValue($value) . '~' . ($levenshteinDistance === null ? '' : $this->processFloat($levenshteinDistance));
$this->predicates[] = $this->processValue($value) . '~' .
($levenshteinDistance === null ? '' : $this->processFloat($levenshteinDistance));

return $this;
}
Expand Down Expand Up @@ -511,22 +521,23 @@ private function traverse(Node $node)
$query .= ' ' . $node->getOperator() . ' ';
}

if($node->getType() === Node::TYPE_CROTCH) {
$addsParentheses = $node->isNegatingWholeChildren() || ($node !== $this->root && \count($node->getChildren()) > 1);
if ($node->getType() === Node::TYPE_CROTCH) {
$addsParentheses = $node->isNegatingWholeChildren()
|| ($node !== $this->root && \count($node->getChildren()) > 1);

if($node->isNegatingWholeChildren()) {
if ($node->isNegatingWholeChildren()) {
$query .= '-';
}

if($addsParentheses) {
if ($addsParentheses) {
$query .= '(';
}

foreach($node->getChildren() as $child) {
foreach ($node->getChildren() as $child) {
$query .= $this->traverse($child);
}

if($addsParentheses) {
if ($addsParentheses) {
$query .= ')';
}
} else {// if ($node->getType() === Node::TYPE_LEAF) { is always true
Expand All @@ -552,10 +563,10 @@ private function traverse(Node $node)
}

$i = 0;
while($i < $countPredicates) {
while ($i < $countPredicates) {
$query .= $criteria->predicates[$i];

if(($i + 1) !== $countPredicates) {
if (($i + 1) !== $countPredicates) {
$query .= ' ';// field:(a b c d...) This falls upon the solr q.op param (default OR)
}

Expand All @@ -580,23 +591,23 @@ private function traverse(Node $node)
*/
private function processValue($value)
{
if($value === '*') {
if ($value === '*') {
return $value;
}

if($value instanceof DateTime) {
if ($value instanceof DateTime) {
$value = $value->format('Y-m-d\TH:i:s\Z');
}

if(\is_bool($value)) {
if (\is_bool($value)) {
return $value ? 'true' : 'false';
}

// Sanitize special chars
$value = \preg_replace('/(\+|-|&&|\|\||!|\(|\)|\{|}|\[|]|\^|"|~|\*|\?|:|\/|\\\)/', '\\\$1', (string) $value);

// Sanitize multiple words
if(\strpos($value, ' ')){
if (\strpos($value, ' ')) {
$value = '"' . $value . '"';
}

Expand All @@ -621,7 +632,9 @@ private function processFloat($value)
private function assertNotBlanks($value)
{
if (\strpos($value, ' ')) {
throw new InvalidArgumentException('Cannot construct query with white spaces. Use expression or multitple clauses instead.');
throw new InvalidArgumentException(
'Cannot construct query with white spaces. Use expression or multitple clauses instead.'
);
}
}

Expand All @@ -635,4 +648,4 @@ private function assertPositiveFloat($distance)
throw new InvalidArgumentException('Distance must not be negative.');
}
}
}
}
12 changes: 6 additions & 6 deletions src/Minimalcode/Search/Internal/Node.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@

/**
* Node tree rappresentation for Criteria.
*
*
* This tree-like logic is needeed for features like nesting and connecting.
*
*
* The class is actually a clear abstract+impls architecture, but for the lack of
* private classes in PHP language (and to keep the classes count low) the
* implementation is just marked as internal and switched at runtime through
* implementation is just marked as internal and switched at runtime through
* the TYPE property.
*
*
* @author Fabio Piro
* @internal
*/
Expand Down Expand Up @@ -113,7 +113,7 @@ public function append($operator, Criteria $criteria)

/**
* Connects the children to a new parent tree.
*
*
* @return $this
*/
public function connect()
Expand Down Expand Up @@ -182,4 +182,4 @@ public function getOperator()
{
return $this->operator;
}
}
}
14 changes: 7 additions & 7 deletions tests/Minimalcode/Search/CriteriaReadmeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,23 +139,23 @@ public function testExpressionReadme()

public function testGeoSearchReadme()
{
self::assertEquals('{!bbox pt=38.116181,\\-86.929463 sfield=position d=100.5}', (string) Criteria::where('position')->nearCircle(38.116181, -86.929463, 100.5));
self::assertEquals('{!geofilt pt=38.116181,\-86.929463 sfield=position d=100.5}', (string) Criteria::where('position')->withinCircle(38.116181, -86.929463, 100.5));
self::assertEquals('position:[38.116181,\-86.929463 TO 38.116181,\-86.929463]', (string) Criteria::where('position')->withinBox(38.116181, -86.929463, 38.116181, -86.929463));
self::assertEquals('{!bbox pt=38.116181,-86.929463 sfield=position d=100.5}', (string) Criteria::where('position')->nearCircle(38.116181, -86.929463, 100.5));
self::assertEquals('{!geofilt pt=38.116181,-86.929463 sfield=position d=100.5}', (string) Criteria::where('position')->withinCircle(38.116181, -86.929463, 100.5));
self::assertEquals('position:[38.116181,-86.929463 TO 38.116181,-86.929463]', (string) Criteria::where('position')->withinBox(38.116181, -86.929463, 38.116181, -86.929463));
}

public function testChristmasReadme()
{
$nearNorthPole = Criteria::where('position')->nearCircle(38.116181, -86.929463, 100.5);
self::assertEquals("{!bbox pt=38.116181,\\-86.929463 sfield=position d=100.5}", $nearNorthPole->getQuery());
self::assertEquals('{!bbox pt=38.116181,-86.929463 sfield=position d=100.5}', $nearNorthPole->getQuery());

$santaClaus = Criteria::where('santa-name')->contains(['Noel', 'Claus', 'Natale', 'Baba', 'Nicolas'])
->andWhere('santa-beard-exists')->is(true)
->andWhere('santa-beard-lenght')->between(5.5, 10.0)
->andWhere('santa-beard-color')->startsWith('whi')->endsWith('te')
->andWhere($nearNorthPole);

self::assertEquals("santa-name:(*Noel* *Claus* *Natale* *Baba* *Nicolas*) AND santa-beard-exists:true AND santa-beard-lenght:[5.5 TO 10] AND santa-beard-color:(whi* *te) AND {!bbox pt=38.116181,\\-86.929463 sfield=position d=100.5}", $santaClaus->getQuery());
self::assertEquals('santa-name:(*Noel* *Claus* *Natale* *Baba* *Nicolas*) AND santa-beard-exists:true AND santa-beard-lenght:[5.5 TO 10] AND santa-beard-color:(whi* *te) AND {!bbox pt=38.116181,-86.929463 sfield=position d=100.5}', $santaClaus->getQuery());

$goodPeople = Criteria::where('good-actions')->greaterThanEqual(10)
->orWhere('bad-actions')->lessThanEqual(5);
Expand All @@ -180,7 +180,7 @@ public function testChristmasReadme()
->orWhere($goodPeople)
);

self::assertEquals("-gift-received:[* TO *] AND chimney:[* TO *] AND date:(2016\\-12\\-25T00\\:00\\:00Z [1970\\-01\\-01T00\\:00\\:00Z TO *]) AND (santa-name:(*Noel* *Claus* *Natale* *Baba* *Nicolas*) AND santa-beard-exists:true AND santa-beard-lenght:[5.5 TO 10] AND santa-beard-color:(whi* *te) AND {!bbox pt=38.116181,\\-86.929463 sfield=position d=100.5}) AND (gift-name:\"LED TV GoPro Oculus Tablet Laptop\"~2 AND gift-type:(information~0.4 tech*) AND __query__:{!dismax qf=myfield}how now brown cow) AND (name:(Christoph Philipp Francisco Fabio)^2.0 OR (good-actions:[10 TO *] OR bad-actions:[* TO 5]))", $giftReceivers->getQuery());
self::assertEquals("-gift-received:[* TO *] AND chimney:[* TO *] AND date:(2016\\-12\\-25T00\\:00\\:00Z [1970\\-01\\-01T00\\:00\\:00Z TO *]) AND (santa-name:(*Noel* *Claus* *Natale* *Baba* *Nicolas*) AND santa-beard-exists:true AND santa-beard-lenght:[5.5 TO 10] AND santa-beard-color:(whi* *te) AND {!bbox pt=38.116181,-86.929463 sfield=position d=100.5}) AND (gift-name:\"LED TV GoPro Oculus Tablet Laptop\"~2 AND gift-type:(information~0.4 tech*) AND __query__:{!dismax qf=myfield}how now brown cow) AND (name:(Christoph Philipp Francisco Fabio)^2.0 OR (good-actions:[10 TO *] OR bad-actions:[* TO 5]))", $giftReceivers->getQuery());
}
}

Expand All @@ -190,4 +190,4 @@ public function __toString()
{
return 'value from __toString';
}
}
}
8 changes: 4 additions & 4 deletions tests/Minimalcode/Search/CriteriaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public function testNearNegativePosition()
$criteria = Criteria::where('field_1')->withinBox(-48.303056, -14.290556, -48.303056, -14.290556);

self::assertEquals('field_1', $this->getField($criteria));
self::assertEquals("field_1:[\\-48.303056,\\-14.290556 TO \\-48.303056,\\-14.290556]", $criteria->getQuery());
self::assertEquals('field_1:[-48.303056,-14.290556 TO -48.303056,-14.290556]', $criteria->getQuery());
self::assertCount(1, $this->getPredicates($criteria));
}

Expand All @@ -20,7 +20,7 @@ public function testWithinCircleNegativePosition()
$criteria = Criteria::where('field_1')->withinCircle(-48.303056, -14.290556, 5);

self::assertEquals('field_1', $this->getField($criteria));
self::assertEquals("{!geofilt pt=\\-48.303056,\\-14.290556 sfield=field_1 d=5.0}", $criteria->getQuery());
self::assertEquals('{!geofilt pt=-48.303056,-14.290556 sfield=field_1 d=5.0}', $criteria->getQuery());
self::assertCount(1, $this->getPredicates($criteria));
}

Expand All @@ -29,7 +29,7 @@ public function testWithinCircleWithBigPosition()
$criteria = Criteria::where('field_1')->withinCircle(-48.303056, -14.290556, 5.123456);

self::assertEquals('field_1', $this->getField($criteria));
self::assertEquals("{!geofilt pt=\\-48.303056,\\-14.290556 sfield=field_1 d=5.123456}", $criteria->getQuery());
self::assertEquals('{!geofilt pt=-48.303056,-14.290556 sfield=field_1 d=5.123456}', $criteria->getQuery());
self::assertCount(1, $this->getPredicates($criteria));
}

Expand Down Expand Up @@ -193,4 +193,4 @@ public function testIsWithNull()
self::assertEquals('-field_1:[* TO *]', $criteria->getQuery());
self::assertCount(1, $this->getPredicates($criteria));
}
}
}
Loading

0 comments on commit 7cd5c61

Please sign in to comment.