Resolver relies on getPrimaryResourceIdentifier() being unique for each schema #11

Activity

1. JanTvrdik commented on Apr 10, 2016

   

   JanTvrdik

   on Apr 10, 2016

   ContributorAuthor

   More actions

   Maybe we can just put sth like this to Validator::validate

   if ($schemaUri === '') {
       $schemaUri = '/schema/' . md5(spl_object_hash($schema));
   }
2. stefk commented on Apr 10, 2016

   

   stefk

   on Apr 10, 2016

   Owner

   More actions

   Unless a $ref pointing to another document is encountered, we never leave the current schema, so it's expected to get always the same identifier (here the empty string) and the same schema from the cache. If an external $ref is met, the identifier returned by getPrimaryResourceIdentifier() must be different. Isn't this the case? Can you provide a schema illustrating the issue?
3. JanTvrdik commented on Apr 10, 2016

   

   JanTvrdik

   on Apr 10, 2016

   ContributorAuthor

   More actions

   I guess you're right. The registerSchema behavior is wrong (relying on uniqueness of primary resource identifier even when it's empty string), but resolve does never actually read those schemas with empty identifier so it never actually breaks anything.

   Therefore the best solution would likely be to just ignore empty identifiers in registerSchema as caching them is both wrong and pointless.

   $id = $uri->getPrimaryResourceIdentifier();
   if ($id !== '' && !isset($this->schemas[$id])) {
       $this->schemas[$id] = $schema;
   }
4. JanTvrdik

   added a commit that references this issue on Apr 10, 2016

   Ignore empty primary resource identifier in Resolve::registerSchema

   ...

   This issue will close once commit a8c6109 is merged into the 'master' branch.

   a8c6109
5. stefk commented on Apr 10, 2016

   

   stefk

   on Apr 10, 2016

   Owner

   More actions

   The registerSchema behavior is wrong (relying on uniqueness of primary resource identifier even when it's empty string)

   I don't agree. The empty string is also a valid identifier. It's unique and will always denote the document from which we started, in case no path/url was provided.

   resolve does never actually read those schemas

   It will when a solution will be provided for #6.
6. JanTvrdik commented on Apr 10, 2016

   

   JanTvrdik

   on Apr 10, 2016

   ContributorAuthor

   More actions

   The collision with #6 is a good observation.

   It's unique and will always denote the document from which we started

   When you call validate with schema A without URL and then validate with schema B without URL, references such as {"$ref": "#"} in schema B would resolve against schema A.
7. stefk commented on Apr 10, 2016

   

   stefk

   on Apr 10, 2016

   Owner

   More actions

   You're right, and actually even for #6, the schema should probably be taken from $stack and not $schemas, so we can safely exclude the empty string as you suggested.
8. JanTvrdik commented on Apr 10, 2016

   

   JanTvrdik

   on Apr 10, 2016

   ContributorAuthor

   More actions

   What about implementing check in registerSchema to prevent accidental overrides

   public function registerSchema(stdClass $schema, Uri $uri)
   {
       $identifier = $uri->getPrimaryResourceIdentifier();
   
       if ($identifier === '') {
           // ignore        
       } elseif (!isset($this->schemas[$identifier])) {
           $this->schemas[$identifier] = $schema;
       } elseif ($this->schemas[$identifier] != $schema) { // intentionally ==, instead of ===
           throw new \LogicException('Different schema is already registered with given URI.');
       }
   }