Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve handling of fragments #382

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- Rescue URI error when initializing a data string that contains a colon
- Fragments with an odd number of components no longer raise an `undefined method `validate'`
error
- Incorrect instantiation of json documents as schemas when resolving fragments
- Validation of a schema with validate_schema and a fragment validates the schema at the fragment pointer
- Handling of fragment strings containing URI escaped values and pointers escaped with ~0 / ~1

## [2.8.0] - 2017-02-07

Expand Down
130 changes: 130 additions & 0 deletions lib/json-schema/pointer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
require 'addressable/uri'

module JSON
class Schema
# a JSON Pointer, as described by RFC 6901 https://tools.ietf.org/html/rfc6901
class Pointer
class Error < JSON::Schema::SchemaError
end
class PointerSyntaxError < Error
end
class ReferenceError < Error
end

# parse a fragment to an array of reference tokens
#
# #/foo/bar
#
# => ['foo', 'bar']
#
# #/foo%20bar
#
# => ['foo bar']
def self.parse_fragment(fragment)
fragment = Addressable::URI.unescape(fragment)
match = fragment.match(/\A#/)
if match
parse_pointer(match.post_match)
else
raise(PointerSyntaxError, "Invalid fragment syntax in #{fragment.inspect}: fragment must begin with #")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also tried to improve the information given in these error messages, as the previous "Invalid fragment resolution for :fragment option" was not very useful to debug

end
end

# parse a pointer to an array of reference tokens
#
# /foo
#
# => ['foo']
#
# /foo~0bar/baz~1qux
#
# => ['foo~bar', 'baz/qux']
def self.parse_pointer(pointer_string)
tokens = pointer_string.split('/', -1).map! do |piece|
piece.gsub('~1', '/').gsub('~0', '~')
end
if tokens[0] == ''
tokens[1..-1]
elsif tokens.empty?
tokens
else
raise(PointerSyntaxError, "Invalid pointer syntax in #{pointer_string.inspect}: pointer must begin with /")
end
end

# initializes a JSON::Schema::Pointer from the given representation.
#
# type may be one of:
#
# - :fragment - the representation is a fragment containing a pointer (starting with #)
# - :pointer - the representation is a pointer (starting with /)
# - :reference_tokens - the representation is an array of tokens referencing a path in a document
def initialize(type, representation)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the more I think about this the more I feel that this aught to be refactored from a single Pointer class, into a Pointer parent class and a subclass for each type (eg. FragmentPointer and AbsolutePointer maybe?). Right now most instance methods work differently based on the type, and it feels like an instance of the Switch Statements Smell, where polymorphism would work better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started down this path but on further reflection I think I disagree. the class represents the same sort of pointer no matter what the external representation it came from. the only method that is any different, once the external representation has been parsed, is #to_s and that is only to keep track of where it came from - it's not really a part of what the Pointer is, just something I found useful for debugging.

@type = type
if type == :reference_tokens
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see anywhere where this is used. Is it needed? (And if not we can remove the last clause in representation_s too)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used it in code that I have been working on that relies on this Pointer class - I use the array of reference tokens there. dealing with the string (pointer or fragment) in code is less useful than the tokens. I'd like this to be part of the API of this class.

reference_tokens = representation
elsif type == :fragment
reference_tokens = self.class.parse_fragment(representation)
elsif type == :pointer
reference_tokens = self.class.parse_pointer(representation)
else
raise ArgumentError, "invalid initialization type: #{type.inspect} with representation #{representation.inspect}"
end
@reference_tokens = reference_tokens.map(&:freeze).freeze
end

attr_reader :reference_tokens

# takes a root json document and evaluates this pointer through the document, returning the value
# pointed to by this pointer.
def evaluate(document)
reference_tokens.inject(document) do |value, token|
if value.is_a?(Array)
if token.is_a?(String) && token =~ /\A\d|[1-9]\d+\z/
token = token.to_i
end
unless token.is_a?(Integer)
raise(ReferenceError, "Invalid resolution for #{to_s}: #{token.inspect} is not an integer and cannot be resolved in array #{value.inspect}")
end
unless (0...value.size).include?(token)
raise(ReferenceError, "Invalid resolution for #{to_s}: #{token.inspect} is not a valid index of #{value.inspect}")
end
elsif value.is_a?(Hash)
unless value.key?(token)
raise(ReferenceError, "Invalid resolution for #{to_s}: #{token.inspect} is not a valid key of #{value.inspect}")
end
else
raise(ReferenceError, "Invalid resolution for #{to_s}: #{token.inspect} cannot be resolved in #{value.inspect}")
end
value[token]
end
end

# the pointer string representation of this Pointer
def pointer
reference_tokens.map { |t| '/' + t.to_s.gsub('~', '~0').gsub('/', '~1') }.join('')
end

# the fragment string representation of this Pointer
def fragment
'#' + Addressable::URI.escape(pointer)
end

def to_s
"#<#{self.class.name} #{@type} = #{representation_s}>"
end

private

def representation_s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make this and the methods below private, to remove them from the public api of the class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made this method private but #pointer and #fragment should be public. I've added method documents for those.

if @type == :fragment
fragment
elsif @type == :pointer
pointer
else
reference_tokens.inspect
end
end
end
end
end
6 changes: 5 additions & 1 deletion lib/json-schema/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ class Schema
attr_accessor :schema, :uri, :validator

def initialize(schema,uri,parent_validator=nil)
@schema = schema
unless schema.respond_to?(:to_hash)
raise ArgumentError, "schema must be a hash; got: #{schema.inspect}"
end

@schema = schema.to_hash
@uri = uri

# If there is an ID on this schema, use it to generate the URI
Expand Down
38 changes: 8 additions & 30 deletions lib/json-schema/validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
require 'json-schema/errors/json_load_error'
require 'json-schema/errors/json_parse_error'
require 'json-schema/util/uri'
require 'json-schema/pointer'

module JSON

Expand Down Expand Up @@ -55,6 +56,11 @@ def initialize(schema_data, data, opts={})
@data = initialize_data(data)
@@mutex.synchronize { build_schemas(@base_schema) }

# If the :fragment option is set, try and validate against the fragment
if opts[:fragment]
@base_schema = schema_from_fragment(@base_schema, opts[:fragment])
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is moved before the validate_schema block so that the correct schema, at the fragment pointer, is validated


# validate the schema, if requested
if @options[:validate_schema]
if @base_schema.schema["$schema"]
Expand All @@ -64,42 +70,14 @@ def initialize(schema_data, data, opts={})
# Don't clear the cache during metaschema validation!
self.class.validate!(metaschema, @base_schema.schema, {:clear_cache => false})
end

# If the :fragment option is set, try and validate against the fragment
if opts[:fragment]
@base_schema = schema_from_fragment(@base_schema, opts[:fragment])
end
end

def schema_from_fragment(base_schema, fragment)
schema_uri = base_schema.uri
fragments = fragment.split("/")
Copy link
Contributor Author

@notEthan notEthan May 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another fix I just noticed: this should be split("/", -1); as it is empty string keys would be mishandled. #/foo// should parse to fragments ['foo', '', ''] and in json {"foo": {"": {"": 7}}} should evaluate to 7.


# ensure the first element was a hash, per the fragment spec
if fragments.shift != "#"
raise JSON::Schema::SchemaError.new("Invalid fragment syntax in :fragment option")
end
pointer = JSON::Schema::Pointer.new(:fragment, fragment)

fragments.each do |f|
if base_schema.is_a?(JSON::Schema) #test if fragment is a JSON:Schema instance
if !base_schema.schema.has_key?(f)
raise JSON::Schema::SchemaError.new("Invalid fragment resolution for :fragment option")
end
base_schema = base_schema.schema[f]
elsif base_schema.is_a?(Hash)
if !base_schema.has_key?(f)
raise JSON::Schema::SchemaError.new("Invalid fragment resolution for :fragment option")
end
base_schema = JSON::Schema.new(base_schema[f],schema_uri,@options[:version])
elsif base_schema.is_a?(Array)
if base_schema[f.to_i].nil?
raise JSON::Schema::SchemaError.new("Invalid fragment resolution for :fragment option")
end
base_schema = JSON::Schema.new(base_schema[f.to_i],schema_uri,@options[:version])
else
raise JSON::Schema::SchemaError.new("Invalid schema encountered when resolving :fragment option")
end
end
base_schema = JSON::Schema.new(pointer.evaluate(base_schema.schema), schema_uri, @options[:version])

if @options[:list]
base_schema.to_array_schema
Expand Down
1 change: 1 addition & 0 deletions test/caching_test.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require File.expand_path('../support/test_helper', __FILE__)
require 'tempfile'

class CachingTestTest < Minitest::Test
def setup
Expand Down
59 changes: 57 additions & 2 deletions test/fragment_resolution_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ def test_fragment_resolution
refute_valid schema, data
assert_valid schema, data, :fragment => "#/properties/a"

assert_raises JSON::Schema::SchemaError do
assert_raises JSON::Schema::Pointer::PointerSyntaxError do
JSON::Validator.validate!(schema,data,:fragment => "/properties/a")
end

assert_raises JSON::Schema::SchemaError do
assert_raises JSON::Schema::Pointer::ReferenceError do
JSON::Validator.validate!(schema,data,:fragment => "#/properties/b")
end
end
Expand Down Expand Up @@ -60,6 +60,34 @@ def test_even_level_fragment_resolution
refute_valid schema, {}, :fragment => "#/foo/bar"
end

def test_fragment_resolution_with_array
document = {
"definitions" => {
"schemas" => [
{
"type" => "object",
"required" => ["a"],
"properties" => {
"a" => {
"type" => "object",
}
}
},
{
"type" => "object",
"properties" => {
"b" => {"type" => "integer" }
}
}
]
}
}

data = {"b" => 5}
refute_valid document, data, :fragment => "#/definitions/schemas/0"
assert_valid document, data, :fragment => "#/definitions/schemas/1"
end

def test_array_fragment_resolution
schema = {
"type" => "object",
Expand All @@ -80,4 +108,31 @@ def test_array_fragment_resolution
assert_valid schema, 5, :fragment => "#/properties/a/anyOf/0"
refute_valid schema, 5, :fragment => "#/properties/a/anyOf/1"
end
def test_fragment_resolution_with_special_chars
document = {
"de~fi/nitions" => {
"schemas" => [
{
"type" => "object",
"required" => ["a"],
"properties" => {
"a" => {
"type" => "object",
}
}
},
{
"type" => "object",
"properties" => {
"b" => {"type" => "integer" }
}
}
]
}
}

data = {"b" => 5}
refute_valid document, data, :fragment => "#/de~0fi~1nitions/schemas/0"
assert_valid document, data, :fragment => "#/de~0fi~1nitions/schemas/1"
end
end
44 changes: 44 additions & 0 deletions test/json_schema_pointer_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
require File.expand_path('../support/test_helper', __FILE__)
require 'json-schema/pointer'

class JsonSchemaPointerTest < Minitest::Test
def test_initialize_parsing_fragment
pointer = JSON::Schema::Pointer.new(:fragment, "#/a%2520%20b/c~1d/e%7E0f/0")

assert_equal(['a%20 b', 'c/d', 'e~f', '0'], pointer.reference_tokens)
end
def test_initialize_parsing_pointer
pointer = JSON::Schema::Pointer.new(:pointer, "/a%20 b/c~1d/e~0f/0")

assert_equal(['a%20 b', 'c/d', 'e~f', '0'], pointer.reference_tokens)
end
def test_initialize_reference_tokens
pointer = JSON::Schema::Pointer.new(:reference_tokens, ['a%20 b', 'c/d', 'e~f', '0'])

assert_equal(['a%20 b', 'c/d', 'e~f', '0'], pointer.reference_tokens)
end
def test_initialize_bad_fragment
assert_raises(JSON::Schema::Pointer::PointerSyntaxError) do
JSON::Schema::Pointer.new(:fragment, "a%2520%20b/c~1d/e%7E0f/0")
end
end
def test_initialize_bad_pointer
assert_raises(JSON::Schema::Pointer::PointerSyntaxError) do
JSON::Schema::Pointer.new(:pointer, "a%20 b/c~1d/e~0f/0")
end
end
def test_evaluate_success
pointer = JSON::Schema::Pointer.new(:fragment, "#/a%2520%20b/c~1d/e%7E0f/0")
assert_equal(1, pointer.evaluate({'a%20 b' => {'c/d' => {'e~f' => [1]}}}))
end
def test_evaluate_empty_strings_success
pointer = JSON::Schema::Pointer.new(:fragment, "#/a///0//")
assert_equal(1, pointer.evaluate({'a' => {'' => {'' => [{'' => {'' => 1}}]}}}))
end
def test_evaluate_fail
assert_raises(JSON::Schema::Pointer::ReferenceError) do
pointer = JSON::Schema::Pointer.new(:fragment, "#/a%2520%20b/c~1d/e%7E0f/0")
pointer.evaluate([])
end
end
end