From cb52269d6e0f5ff64888d5f377ae2741cda32694 Mon Sep 17 00:00:00 2001 From: Iain Beeston Date: Fri, 6 Mar 2015 11:58:49 +0000 Subject: [PATCH 1/2] Only rescue errors explicitly There are a lot of places in the code where we rescue any error, at all. That's quite dangerous, as it could conceal bugs. I've changed it so that we always specify which error class we want to catch. Mostly it's been a minor change, but there are two API changes: * When it comes to loading data I've had to introduce an explicit list of protocols which we can load json over (otherwise it's possible to have a uri with a scheme that makes no sense - it'd still be a valid uri but loading from it is impossible). I've used the list of protocols that addressable recognizes for now. * No matter what JSON parser you use, we now always raise a JSON::Schema::JsonParseError. Without doing this it would be tricky to handle parser errors identically, for all parsers (the other option would be to catch a long list of potential parse errors, but this seems more sensible). --- lib/json-schema/attributes/formats/date.rb | 3 +- .../attributes/formats/date_time.rb | 14 ++-- lib/json-schema/attributes/not.rb | 2 +- lib/json-schema/errors/json_load_error.rb | 6 ++ lib/json-schema/errors/schema_parse_error.rb | 8 +++ lib/json-schema/schema/reader.rb | 4 +- lib/json-schema/util/uri.rb | 2 + lib/json-schema/validator.rb | 64 ++++++++++++------- test/test_helper.rb | 10 --- test/test_initialize_data.rb | 22 ++++--- test/test_schema_loader.rb | 2 +- 11 files changed, 83 insertions(+), 54 deletions(-) create mode 100644 lib/json-schema/errors/json_load_error.rb create mode 100644 lib/json-schema/errors/schema_parse_error.rb diff --git a/lib/json-schema/attributes/formats/date.rb b/lib/json-schema/attributes/formats/date.rb index 24b21700..451f6ff1 100644 --- a/lib/json-schema/attributes/formats/date.rb +++ b/lib/json-schema/attributes/formats/date.rb @@ -11,7 +11,8 @@ def self.validate(current_schema, data, fragments, processor, validator, options if REGEXP.match(data) begin Date.parse(data) - rescue Exception + rescue ArgumentError => e + raise e unless e.message == 'invalid date' validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) end else diff --git a/lib/json-schema/attributes/formats/date_time.rb b/lib/json-schema/attributes/formats/date_time.rb index b1baa15c..01987485 100644 --- a/lib/json-schema/attributes/formats/date_time.rb +++ b/lib/json-schema/attributes/formats/date_time.rb @@ -14,18 +14,16 @@ def self.validate(current_schema, data, fragments, processor, validator, options begin Date.parse(parts[0]) - rescue Exception + rescue ArgumentError => e + raise e unless e.message == 'invalid date' validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) return end - begin - validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) and return if m[1].to_i > 23 - validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) and return if m[2].to_i > 59 - validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) and return if m[3].to_i > 59 - rescue Exception - validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) - end + validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) and return if m.length < 4 + validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) and return if m[1].to_i > 23 + validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) and return if m[2].to_i > 59 + validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) and return if m[3].to_i > 59 else validation_error(processor, error_message, fragments, current_schema, self, options[:record_errors]) end diff --git a/lib/json-schema/attributes/not.rb b/lib/json-schema/attributes/not.rb index 52729ef3..64fbd6e3 100644 --- a/lib/json-schema/attributes/not.rb +++ b/lib/json-schema/attributes/not.rb @@ -17,7 +17,7 @@ def self.validate(current_schema, data, fragments, processor, validator, options message = "The property '#{build_fragment(fragments)}' of type #{data.class} matched the disallowed schema" failed = false end - rescue + rescue ValidationError # Yay, we failed validation. end diff --git a/lib/json-schema/errors/json_load_error.rb b/lib/json-schema/errors/json_load_error.rb new file mode 100644 index 00000000..f5a74b78 --- /dev/null +++ b/lib/json-schema/errors/json_load_error.rb @@ -0,0 +1,6 @@ +module JSON + class Schema + class JsonLoadError < StandardError + end + end +end diff --git a/lib/json-schema/errors/schema_parse_error.rb b/lib/json-schema/errors/schema_parse_error.rb new file mode 100644 index 00000000..afdf84f1 --- /dev/null +++ b/lib/json-schema/errors/schema_parse_error.rb @@ -0,0 +1,8 @@ +require 'json/common' + +module JSON + class Schema + class SchemaParseError < JSON::ParserError + end + end +end diff --git a/lib/json-schema/schema/reader.rb b/lib/json-schema/schema/reader.rb index 9b13c66b..4121bf99 100644 --- a/lib/json-schema/schema/reader.rb +++ b/lib/json-schema/schema/reader.rb @@ -57,8 +57,8 @@ def initialize(options = {}) # @param location [#to_s] The location from which to read the schema # @return [JSON::Schema] # @raise [JSON::Schema::ReadRefused] if +accept_uri+ or +accept_file+ - # indicated the schema should not be readed - # @raise [JSON::ParserError] if the schema was not a valid JSON object + # indicated the schema could not be read + # @raise [JSON::Schema::ParseError] if the schema was not a valid JSON object def read(location) uri = Addressable::URI.parse(location.to_s) body = if uri.scheme.nil? || uri.scheme == 'file' diff --git a/lib/json-schema/util/uri.rb b/lib/json-schema/util/uri.rb index eca9e6ed..951c2ce9 100644 --- a/lib/json-schema/util/uri.rb +++ b/lib/json-schema/util/uri.rb @@ -1,6 +1,8 @@ module JSON module Util module URI + SUPPORTED_PROTOCOLS = %w(http https ftp tftp sftp ssh svn+ssh telnet nntp gopher wais ldap prospero) + def self.normalized_uri(uri) uri = Addressable::URI.parse(uri) unless uri.is_a?(Addressable::URI) # Check for absolute path diff --git a/lib/json-schema/validator.rb b/lib/json-schema/validator.rb index c20971b7..0adb1e8d 100644 --- a/lib/json-schema/validator.rb +++ b/lib/json-schema/validator.rb @@ -9,6 +9,8 @@ require 'json-schema/schema/reader' require 'json-schema/errors/schema_error' +require 'json-schema/errors/schema_parse_error' +require 'json-schema/errors/json_load_error' require 'json-schema/errors/json_parse_error' module JSON @@ -54,17 +56,13 @@ def initialize(schema_data, data, opts={}) # validate the schema, if requested if @options[:validate_schema] - begin - if @base_schema.schema["$schema"] - base_validator = JSON::Validator.validator_for_name(@base_schema.schema["$schema"]) - end - metaschema = base_validator ? base_validator.metaschema : validator.metaschema - # Don't clear the cache during metaschema validation! - meta_validator = JSON::Validator.new(metaschema, @base_schema.schema, {:clear_cache => false}) - meta_validator.validate - rescue JSON::Schema::ValidationError, JSON::Schema::SchemaError - raise $! + if @base_schema.schema["$schema"] + base_validator = JSON::Validator.validator_for_name(@base_schema.schema["$schema"]) end + metaschema = base_validator ? base_validator.metaschema : validator.metaschema + # Don't clear the cache during metaschema validation! + meta_validator = JSON::Validator.new(metaschema, @base_schema.schema, {:clear_cache => false}) + meta_validator.validate end # If the :fragment option is set, try and validate against the fragment @@ -415,15 +413,27 @@ def json_backend=(backend) def parse(s) if defined?(MultiJson) - MultiJson.respond_to?(:adapter) ? MultiJson.load(s) : MultiJson.decode(s) + begin + MultiJson.respond_to?(:adapter) ? MultiJson.load(s) : MultiJson.decode(s) + rescue MultiJson::ParseError => e + raise JSON::Schema::JsonParseError.new(e.message) + end else case @@json_backend.to_s when 'json' - JSON.parse(s, :quirks_mode => true) + begin + JSON.parse(s, :quirks_mode => true) + rescue JSON::ParserError => e + raise JSON::Schema::JsonParseError.new(e.message) + end when 'yajl' - json = StringIO.new(s) - parser = Yajl::Parser.new - parser.parse(json) or raise JSON::Schema::JsonParseError.new("The JSON could not be parsed by yajl") + begin + json = StringIO.new(s) + parser = Yajl::Parser.new + parser.parse(json) or raise JSON::Schema::JsonParseError.new("The JSON could not be parsed by yajl") + rescue Yajl::ParseError => e + raise JSON::Schema::JsonParseError.new(e.message) + end else raise JSON::Schema::JsonParseError.new("No supported JSON parsers found. The following parsers are suported:\n * yajl-ruby\n * json") end @@ -520,7 +530,7 @@ def initialize_schema(schema) schema = schema.to_array_schema end Validator.add_schema(schema) - rescue + rescue JSON::Schema::JsonParseError # Build a uri for it schema_uri = Util::URI.normalized_uri(schema) if !self.class.schema_loaded?(schema_uri) @@ -551,7 +561,7 @@ def initialize_schema(schema) end Validator.add_schema(schema) else - raise "Invalid schema - must be either a string or a hash" + raise SchemaParseError, "Invalid schema - must be either a string or a hash" end schema @@ -567,12 +577,12 @@ def initialize_data(data) elsif data.is_a?(String) begin data = JSON::Validator.parse(data) - rescue + rescue JSON::Schema::JsonParseError begin json_uri = Util::URI.normalized_uri(data) data = JSON::Validator.parse(custom_open(json_uri)) - rescue - # Silently discard the error - the data will not change + rescue JSON::Schema::JsonLoadError + # Silently discard the error - use the data as-is end end end @@ -582,10 +592,18 @@ def initialize_data(data) def custom_open(uri) uri = Util::URI.normalized_uri(uri) if uri.is_a?(String) - if uri.absolute? && uri.scheme != 'file' - open(uri.to_s).read + if uri.absolute? && Util::URI::SUPPORTED_PROTOCOLS.include?(uri.scheme) + begin + open(uri.to_s).read + rescue OpenURI::HTTPError, Timeout::Error => e + raise JSON::Schema::JsonLoadError, e.message + end else - File.read(Addressable::URI.unescape(uri.path)) + begin + File.read(Addressable::URI.unescape(uri.path)) + rescue SystemCallError => e + raise JSON::Schema::JsonLoadError, e.message + end end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index e8d6aef1..14510863 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -34,14 +34,4 @@ def refute_valid(schema, data, options = {}) errors = JSON::Validator.fully_validate(schema, data, options) refute_equal([], errors, "#{data.inspect} should be invalid for schema:\n#{schema.inspect}") end - - def parser_error - if defined?(::Yajl) - Yajl::ParseError - elsif defined?(::MultiJson) - MultiJson::ParseError - else - JSON::ParserError - end - end end diff --git a/test/test_initialize_data.rb b/test/test_initialize_data.rb index bf61c65f..77d6c112 100644 --- a/test/test_initialize_data.rb +++ b/test/test_initialize_data.rb @@ -10,11 +10,11 @@ def test_parse_character_string assert(JSON::Validator.validate(schema, data, :parse_data => false)) - assert_raises(parser_error) do + assert_raises(JSON::Schema::JsonParseError) do JSON::Validator.validate(schema, data, :json => true) end - assert_raises(Errno::ENOENT) { JSON::Validator.validate(schema, data, :uri => true) } + assert_raises(JSON::Schema::JsonLoadError) { JSON::Validator.validate(schema, data, :uri => true) } end def test_parse_integer_string @@ -27,7 +27,7 @@ def test_parse_integer_string assert(JSON::Validator.validate(schema, data, :json => true)) - assert_raises(Errno::ENOENT) { JSON::Validator.validate(schema, data, :uri => true) } + assert_raises(JSON::Schema::JsonLoadError) { JSON::Validator.validate(schema, data, :uri => true) } end def test_parse_hash_string @@ -40,7 +40,7 @@ def test_parse_hash_string assert(JSON::Validator.validate(schema, data, :json => true)) - assert_raises(Errno::ENOENT) { JSON::Validator.validate(schema, data, :uri => true) } + assert_raises(JSON::Schema::JsonLoadError) { JSON::Validator.validate(schema, data, :uri => true) } end def test_parse_json_string @@ -53,7 +53,7 @@ def test_parse_json_string assert(JSON::Validator.validate(schema, data, :json => true)) - assert_raises(Errno::ENOENT) { JSON::Validator.validate(schema, data, :uri => true) } + assert_raises(JSON::Schema::JsonLoadError) { JSON::Validator.validate(schema, data, :uri => true) } end def test_parse_valid_uri_string @@ -66,7 +66,7 @@ def test_parse_valid_uri_string assert(JSON::Validator.validate(schema, data, :parse_data => false)) - assert_raises(parser_error) do + assert_raises(JSON::Schema::JsonParseError) do JSON::Validator.validate(schema, data, :json => true) end @@ -83,11 +83,17 @@ def test_parse_invalid_uri_string assert(JSON::Validator.validate(schema, data, :parse_data => false)) - assert_raises(parser_error) do + stub_request(:get, "foo.bar").to_return(:status => [500, "Internal Server Error"]) + + assert(JSON::Validator.validate(schema, data)) + + assert(JSON::Validator.validate(schema, data, :parse_data => false)) + + assert_raises(JSON::Schema::JsonParseError) do JSON::Validator.validate(schema, data, :json => true) end - assert_raises(Timeout::Error) { JSON::Validator.validate(schema, data, :uri => true) } + assert_raises(JSON::Schema::JsonLoadError) { JSON::Validator.validate(schema, data, :uri => true) } end def test_parse_integer diff --git a/test/test_schema_loader.rb b/test/test_schema_loader.rb index b267e071..1972c830 100644 --- a/test/test_schema_loader.rb +++ b/test/test_schema_loader.rb @@ -67,7 +67,7 @@ def test_parse_error reader = JSON::Schema::Reader.new - assert_raises(parser_error) do + assert_raises(JSON::Schema::JsonParseError) do reader.read(ADDRESS_SCHEMA_URI) end end From 31a620b7e9e6f7b908e989b365d86a94b75a9b66 Mon Sep 17 00:00:00 2001 From: Iain Beeston Date: Wed, 23 Dec 2015 08:01:21 +0000 Subject: [PATCH 2/2] Added a changelog --- CHANGELOG.md | 12 ++++++++++++ CONTRIBUTING.md | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..e73b7925 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,12 @@ +# Change Log +All notable changes to this project will be documented in this file. +Please keep to the changelog format described on [keepachangelog.com](http://keepachangelog.com). +This project adheres to [Semantic Versioning](http://semver.org/). + +## [Unreleased] +### Added +- Added a changelog + +### Changed +- Made validation failures raise a `JSON::Schema::SchemaParseError` and data + loading failures a `JSON::Schema::JsonLoadError` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ca5c4d57..1b2df766 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,6 +2,6 @@ The Ruby JSON Schema library is meant to be a community effort, and as such, the All individuals that have a pull request merged will receive collaborator access to the repository. Due to the restrictions on RubyGems authentication, permissions to release a gem must be requested along with the email desired to be associated with the release credentials. -Accepting changes to the JSON Schema library shall be made through the use of pull requests on GitHub. A pull request must receive at least two (2) "+1" comments from current contributors to the JSON Schema library before being accepted and merged. If a breaking issue and fix exists, please feel free to contact the project maintainer at hoxworth@gmail.com or @hoxworth for faster resolution. +Accepting changes to the JSON Schema library shall be made through the use of pull requests on GitHub. A pull request must receive at least two (2) "+1" comments from current contributors, and include a relevant changelog entry, before being accepted and merged. If a breaking issue and fix exists, please feel free to contact the project maintainer at hoxworth@gmail.com or @hoxworth for faster resolution. Releases follow semantic versioning and may be made at a maintainer's discretion.