Skip to content

Commit

Permalink
Fix infinite recursion on some schemas when setting defaults (#359) (#…
Browse files Browse the repository at this point in the history 
…365)

* Don't try to fetch files that don't exist

Throws an exception when the ref can't be resolved to a useful file URI,
rather than waiting for something further down the line to fail after
the fact.

* Refactor defaults code to use LooseTypeCheck where appropriate

* Test for not treating non-containers like arrays

* Update comments

* Rename variable for clarity

* Add CHECK_MODE_ONLY_REQUIRED_DEFAULTS

If CHECK_MODE_ONLY_REQUIRED_DEFAULTS is set, then only apply defaults
if they are marked as required.

* Workaround for $this scope issue on PHP-5.3

* Fix infinite recursion via $ref when applying defaults

* Add missing second test for array case

* Add test for setting a default value for null

* Also fix infinite recursion via $ref for array defaults

* Move nested closure into separate method

* $parentSchema will always be set when $name is, so don't check it

* Handle nulls properly - fixes issue #377
  • Loading branch information
@erayd @bighappyface
erayd authored and bighappyface committed Mar 21, 2017
1 parent af14372 commit 46d10ac
Show file tree
Hide file tree
Showing 8 changed files with 274 additions and 82 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ third argument to `Validator::validate()`, or can be provided as the third argum
| `Constraint::CHECK_MODE_TYPE_CAST` | Enable fuzzy type checking for associative arrays and objects |
| `Constraint::CHECK_MODE_COERCE_TYPES` | Convert data types to match the schema where possible |
| `Constraint::CHECK_MODE_APPLY_DEFAULTS` | Apply default values from the schema if not set |
| `Constraint::CHECK_MODE_ONLY_REQUIRED_DEFAULTS` | When applying defaults, only set values that are required |
| `Constraint::CHECK_MODE_EXCEPTIONS` | Throw an exception immediately if validation fails |
| `Constraint::CHECK_MODE_DISABLE_FORMAT` | Do not validate "format" constraints |

Expand Down
9 changes: 5 additions & 4 deletions src/JsonSchema/Constraints/Constraint.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ abstract class Constraint extends BaseConstraint implements ConstraintInterface
const CHECK_MODE_APPLY_DEFAULTS = 0x00000008;
const CHECK_MODE_EXCEPTIONS = 0x00000010;
const CHECK_MODE_DISABLE_FORMAT = 0x00000020;
const CHECK_MODE_ONLY_REQUIRED_DEFAULTS = 0x00000080;

/**
* Bubble down the path
Expand Down Expand Up @@ -78,10 +79,10 @@ protected function checkArray(&$value, $schema = null, JsonPointer $path = null,
* @param mixed $i
* @param mixed $patternProperties
*/
protected function checkObject(&$value, $schema = null, JsonPointer $path = null, $i = null, $patternProperties = null)
protected function checkObject(&$value, $schema = null, JsonPointer $path = null, $i = null, $patternProperties = null, $appliedDefaults = array())
{
$validator = $this->factory->createInstanceFor('object');
$validator->check($value, $schema, $path, $i, $patternProperties);
$validator->check($value, $schema, $path, $i, $patternProperties, $appliedDefaults);

$this->addErrors($validator->getErrors());
}
Expand Down Expand Up @@ -110,11 +111,11 @@ protected function checkType(&$value, $schema = null, JsonPointer $path = null,
* @param JsonPointer|null $path
* @param mixed $i
*/
protected function checkUndefined(&$value, $schema = null, JsonPointer $path = null, $i = null)
protected function checkUndefined(&$value, $schema = null, JsonPointer $path = null, $i = null, $fromDefault = false)
{
$validator = $this->factory->createInstanceFor('undefined');

$validator->check($value, $this->factory->getSchemaStorage()->resolveRefSchema($schema), $path, $i);
$validator->check($value, $this->factory->getSchemaStorage()->resolveRefSchema($schema), $path, $i, $fromDefault);

$this->addErrors($validator->getErrors());
}
Expand Down
17 changes: 12 additions & 5 deletions src/JsonSchema/Constraints/ObjectConstraint.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,22 @@
*/
class ObjectConstraint extends Constraint
{
/**
* @var array List of properties to which a default value has been applied
*/
protected $appliedDefaults = array();

/**
* {@inheritdoc}
*/
public function check(&$element, $definition = null, JsonPointer $path = null, $additionalProp = null, $patternProperties = null)
public function check(&$element, $definition = null, JsonPointer $path = null, $additionalProp = null, $patternProperties = null, $appliedDefaults = array())
{
if ($element instanceof UndefinedConstraint) {
return;
}

$this->appliedDefaults = $appliedDefaults;

$matches = array();
if ($patternProperties) {
$matches = $this->validatePatternProperties($element, $path, $patternProperties);
Expand Down Expand Up @@ -64,7 +71,7 @@ public function validatePatternProperties($element, JsonPointer $path = null, $p
foreach ($element as $i => $value) {
if (preg_match($delimiter . $pregex . $delimiter . 'u', $i)) {
$matches[] = $i;
$this->checkUndefined($value, $schema ?: new \stdClass(), $path, $i);
$this->checkUndefined($value, $schema ?: new \stdClass(), $path, $i, in_array($i, $this->appliedDefaults));
}
}
}
Expand Down Expand Up @@ -96,9 +103,9 @@ public function validateElement($element, $matches, $objectDefinition = null, Js
// additional properties defined
if (!in_array($i, $matches) && $additionalProp && !$definition) {
if ($additionalProp === true) {
$this->checkUndefined($value, null, $path, $i);
$this->checkUndefined($value, null, $path, $i, in_array($i, $this->appliedDefaults));
} else {
$this->checkUndefined($value, $additionalProp, $path, $i);
$this->checkUndefined($value, $additionalProp, $path, $i, in_array($i, $this->appliedDefaults));
}
}

Expand Down Expand Up @@ -135,7 +142,7 @@ public function validateDefinition(&$element, $objectDefinition = null, JsonPoin

if (is_object($definition)) {
// Undefined constraint will check for is_object() and quit if is not - so why pass it?
$this->checkUndefined($property, $definition, $path, $i);
$this->checkUndefined($property, $definition, $path, $i, in_array($i, $this->appliedDefaults));
}
}
}
Expand Down
145 changes: 103 additions & 42 deletions src/JsonSchema/Constraints/UndefinedConstraint.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,24 @@
*/
class UndefinedConstraint extends Constraint
{
/**
* @var array List of properties to which a default value has been applied
*/
protected $appliedDefaults = array();

/**
* {@inheritdoc}
*/
public function check(&$value, $schema = null, JsonPointer $path = null, $i = null)
public function check(&$value, $schema = null, JsonPointer $path = null, $i = null, $fromDefault = false)
{
if (is_null($schema) || !is_object($schema)) {
return;
}

$path = $this->incrementPath($path ?: new JsonPointer(''), $i);
if ($fromDefault) {
$path->setFromDefault();
}

// check special properties
$this->validateCommonProperties($value, $schema, $path, $i);
Expand Down Expand Up @@ -68,7 +76,8 @@ public function validateTypes(&$value, $schema = null, JsonPointer $path, $i = n
isset($schema->properties) ? $this->factory->getSchemaStorage()->resolveRefSchema($schema->properties) : $schema,
$path,
isset($schema->additionalProperties) ? $schema->additionalProperties : null,
isset($schema->patternProperties) ? $schema->patternProperties : null
isset($schema->patternProperties) ? $schema->patternProperties : null,
$this->appliedDefaults
);
}

Expand Down Expand Up @@ -113,46 +122,8 @@ protected function validateCommonProperties(&$value, $schema = null, JsonPointer
}

// Apply default values from schema
if ($this->factory->getConfig(self::CHECK_MODE_APPLY_DEFAULTS)) {
if ($this->getTypeCheck()->isObject($value) && isset($schema->properties)) {
// $value is an object, so apply default properties if defined
foreach ($schema->properties as $currentProperty => $propertyDefinition) {
if (!$this->getTypeCheck()->propertyExists($value, $currentProperty) && isset($propertyDefinition->default)) {
if (is_object($propertyDefinition->default)) {
$this->getTypeCheck()->propertySet($value, $currentProperty, clone $propertyDefinition->default);
} else {
$this->getTypeCheck()->propertySet($value, $currentProperty, $propertyDefinition->default);
}
}
}
} elseif ($this->getTypeCheck()->isArray($value)) {
if (isset($schema->properties)) {
// $value is an array, but default properties are defined, so treat as assoc
foreach ($schema->properties as $currentProperty => $propertyDefinition) {
if (!isset($value[$currentProperty]) && isset($propertyDefinition->default)) {
if (is_object($propertyDefinition->default)) {
$value[$currentProperty] = clone $propertyDefinition->default;
} else {
$value[$currentProperty] = $propertyDefinition->default;
}
}
}
} elseif (isset($schema->items)) {
// $value is an array, and default items are defined - treat as plain array
foreach ($schema->items as $currentProperty => $itemDefinition) {
if (!isset($value[$currentProperty]) && isset($itemDefinition->default)) {
if (is_object($itemDefinition->default)) {
$value[$currentProperty] = clone $itemDefinition->default;
} else {
$value[$currentProperty] = $itemDefinition->default;
}
}
}
}
} elseif (($value instanceof self || $value === null) && isset($schema->default)) {
// $value is a leaf, not a container - apply the default directly
$value = is_object($schema->default) ? clone $schema->default : $schema->default;
}
if (!$path->fromDefault()) {
$this->applyDefaultValues($value, $schema, $path);
}

// Verify required values
Expand Down Expand Up @@ -216,6 +187,96 @@ protected function validateCommonProperties(&$value, $schema = null, JsonPointer
}
}

/**
* Check whether a default should be applied for this value
*
* @param mixed $schema
* @param mixed $parentSchema
* @param bool $requiredOnly
*
* @return bool
*/
private function shouldApplyDefaultValue($requiredOnly, $schema, $name = null, $parentSchema = null)
{
// required-only mode is off
if (!$requiredOnly) {
return true;
}
// draft-04 required is set
if (
$name !== null
&& isset($parentSchema->required)
&& is_array($parentSchema->required)
&& in_array($name, $parentSchema->required)
) {
return true;
}
// draft-03 required is set
if (isset($schema->required) && !is_array($schema->required) && $schema->required) {
return true;
}
// default case
return false;
}

/**
* Apply default values
*
* @param mixed $value
* @param mixed $schema
* @param JsonPointer $path
*/
protected function applyDefaultValues(&$value, $schema, $path)
{
// only apply defaults if feature is enabled
if (!$this->factory->getConfig(self::CHECK_MODE_APPLY_DEFAULTS)) {
return;
}

// apply defaults if appropriate
$requiredOnly = $this->factory->getConfig(self::CHECK_MODE_ONLY_REQUIRED_DEFAULTS);
if (isset($schema->properties) && LooseTypeCheck::isObject($value)) {
// $value is an object or assoc array, and properties are defined - treat as an object
foreach ($schema->properties as $currentProperty => $propertyDefinition) {
if (
!LooseTypeCheck::propertyExists($value, $currentProperty)
&& property_exists($propertyDefinition, 'default')
&& $this->shouldApplyDefaultValue($requiredOnly, $propertyDefinition, $currentProperty, $schema)
) {
// assign default value
if (is_object($propertyDefinition->default)) {
LooseTypeCheck::propertySet($value, $currentProperty, clone $propertyDefinition->default);
} else {
LooseTypeCheck::propertySet($value, $currentProperty, $propertyDefinition->default);
}
$this->appliedDefaults[] = $currentProperty;
}
}
} elseif (isset($schema->items) && LooseTypeCheck::isArray($value)) {
// $value is an array, and items are defined - treat as plain array
foreach ($schema->items as $currentItem => $itemDefinition) {
if (
!array_key_exists($currentItem, $value)
&& property_exists($itemDefinition, 'default')
&& $this->shouldApplyDefaultValue($requiredOnly, $itemDefinition)) {
if (is_object($itemDefinition->default)) {
$value[$currentItem] = clone $itemDefinition->default;
} else {
$value[$currentItem] = $itemDefinition->default;
}
}
$path->setFromDefault();
}
} elseif (
$value instanceof self
&& property_exists($schema, 'default')
&& $this->shouldApplyDefaultValue($requiredOnly, $schema)) {
// $value is a leaf, not a container - apply the default directly
$value = is_object($schema->default) ? clone $schema->default : $schema->default;
$path->setFromDefault();
}
}

/**
* Validate allOf, anyOf, and oneOf properties
*
Expand Down
23 changes: 23 additions & 0 deletions src/JsonSchema/Entity/JsonPointer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ class JsonPointer
/** @var string[] */
private $propertyPaths = array();

/**
* @var bool Whether the value at this path was set from a schema default
*/
private $fromDefault = false;

/**
* @param string $value
*
Expand Down Expand Up @@ -135,4 +140,22 @@ public function __toString()
{
return $this->getFilename() . $this->getPropertyPathAsString();
}

/**
* Mark the value at this path as being set from a schema default
*/
public function setFromDefault()
{
$this->fromDefault = true;
}

/**
* Check whether the value at this path was set from a schema default
*
* @return bool
*/
public function fromDefault()
{
return $this->fromDefault;
}
}
12 changes: 11 additions & 1 deletion src/JsonSchema/SchemaStorage.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,18 @@ public function getSchema($id)
public function resolveRef($ref)
{
$jsonPointer = new JsonPointer($ref);
$refSchema = $this->getSchema($jsonPointer->getFilename());

// resolve filename for pointer
$fileName = $jsonPointer->getFilename();
if (!strlen($fileName)) {
throw new UnresolvableJsonPointerException(sprintf(
"Could not resolve fragment '%s': no file is defined",
$jsonPointer->getPropertyPathAsString()
));
}

// get & process the schema
$refSchema = $this->getSchema($fileName);
foreach ($jsonPointer->getPropertyPaths() as $path) {
if (is_object($refSchema) && property_exists($refSchema, $path)) {
$refSchema = $this->resolveRefSchema($refSchema->{$path});
Expand Down
Loading

0 comments on commit 46d10ac

Please sign in to comment.