feat: improve error message when request has non UTF-8 characters (#559)

lib/pact_broker/api/resources/all_webhooks.rb

@@ -30,10 +30,7 @@ def post_is_create?
         end
 
         def malformed_request?
-          if request.post?
-            return invalid_json? || validation_errors?(webhook)
-          end
-          false
+          super || (request.post? && validation_errors?(webhook))
         end
 
         def to_json

lib/pact_broker/api/resources/base_resource.rb

@@ -15,6 +15,7 @@ module PactBroker
   module Api
     module Resources
       class InvalidJsonError < PactBroker::Error ; end
+      class NonUTF8CharacterFound < PactBroker::Error ; end
 
       class BaseResource < Webmachine::Resource
         include PactBroker::Services
@@ -39,6 +40,10 @@ def known_methods
           super + ["PATCH"]
         end
 
+        def malformed_request?
+          content_type_is_json_but_invalid_json_provided?
+        end
+
         def finish_request
           application_context.after_resource&.call(self)
           PactBroker.configuration.after_resource.call(self)
@@ -125,17 +130,32 @@ def params(options = {})
           else
             @params_with_string_keys ||= JSON.parse(request_body, { symbolize_names: false }.merge(PACT_PARSING_OPTIONS)) #Not load! Otherwise it will try to load Ruby classes.
           end
-        rescue JSON::JSONError => e
-          raise InvalidJsonError.new("Error parsing JSON - #{e.message}")
+        rescue StandardError => e
+          fragment = fragment_before_invalid_utf_8_char
+
+          if fragment
+            raise NonUTF8CharacterFound.new(message("errors.non_utf_8_char_in_request_body", char_number: fragment.length + 1, fragment: fragment))
+          else
+            raise InvalidJsonError.new(e.message)
+          end
         end
         # rubocop: enable Metrics/CyclomaticComplexity
 
+        def fragment_before_invalid_utf_8_char
+          request_body.each_char.with_index do | char, index |
+            if !char.valid_encoding?
+              return index < 100 ? request_body[0...index] : request_body[index-100...index]
+            end
+          end
+          nil
+        end
+
         def params_with_string_keys
           params(symbolize_names: false)
         end
 
         def pact_params
-          @pact_params ||= PactBroker::Pacts::PactParams.from_request request, identifier_from_path
+          @pact_params ||= PactBroker::Pacts::PactParams.from_request(request, identifier_from_path)
         end
 
         def set_json_error_message message
@@ -192,9 +212,12 @@ def invalid_json?
           begin
             params
             false
+          rescue NonUTF8CharacterFound => e
+            set_json_error_message(e.message)
+            response.headers["Content-Type"] = "application/hal+json;charset=utf-8"
+            true
           rescue StandardError => e
-            logger.info "Error parsing JSON #{e} - #{request_body}"
-            set_json_error_message "Error parsing JSON - #{e.message}"
+            set_json_error_message("#{e.cause ? e.cause.class.name : e.class.name} - #{e.message}")
             response.headers["Content-Type"] = "application/hal+json;charset=utf-8"
             true
           end
@@ -279,6 +302,10 @@ def validation_errors_for_schema?(schema_to_use = schema, params_to_validate = p
         def malformed_request_for_json_with_schema?(schema_to_use = schema, params_to_validate = params)
           invalid_json? || validation_errors_for_schema?(schema_to_use, params_to_validate)
         end
+
+        def content_type_is_json_but_invalid_json_provided?
+          request.content_type&.include?("json") && any_request_body? && invalid_json?
+        end
       end
     end
   end

lib/pact_broker/api/resources/currently_deployed_versions_for_environment.rb

@@ -8,10 +8,6 @@ module Resources
       class CurrentlyDeployedVersionsForEnvironment < BaseResource
         using PactBroker::StringRefinements
 
-        def content_types_accepted
-          [["application/json", :from_json]]
-        end
-
         def content_types_provided
           [["application/hal+json", :to_json]]
         end

lib/pact_broker/api/resources/currently_supported_versions_for_environment.rb

@@ -8,10 +8,6 @@ module Resources
       class CurrentlySupportedVersionsForEnvironment < BaseResource
         using PactBroker::StringRefinements
 
-        def content_types_accepted
-          [["application/json", :from_json]]
-        end
-
         def content_types_provided
           [["application/hal+json", :to_json]]
         end

lib/pact_broker/api/resources/deployed_version.rb

@@ -8,11 +8,6 @@ module Resources
       class DeployedVersion < BaseResource
         include PactBroker::Messages
 
-        def initialize
-          super
-          @currently_deployed_param = params(default: {})[:currentlyDeployed]
-        end
-
         def content_types_provided
           [
             ["application/hal+json", :to_json]
@@ -33,14 +28,6 @@ def resource_exists?
           !!deployed_version
         end
 
-        def malformed_request?
-          if request.patch?
-            return invalid_json?
-          else
-            false
-          end
-        end
-
         def to_json
           decorator_class(:deployed_version_decorator).new(deployed_version).to_json(decorator_options)
         end
@@ -73,7 +60,14 @@ def policy_record
 
         private
 
-        attr_reader :currently_deployed_param
+        # can't use ||= with a potentially nil value
+        def currently_deployed_param
+          if defined?(@currently_deployed_param)
+            @currently_deployed_param
+          else
+            @currently_deployed_param = params(default: {})[:currentlyDeployed]
+          end
+        end
 
         def process_currently_deployed_param
           if currently_deployed_param == false

lib/pact_broker/api/resources/environment.rb

@@ -22,11 +22,7 @@ def resource_exists?
         end
 
         def malformed_request?
-          if request.put? && environment
-            invalid_json? || validation_errors_for_schema?(schema, params.merge(uuid: uuid))
-          else
-            false
-          end
+          super || (request.put? && environment && validation_errors_for_schema?(schema, params.merge(uuid: uuid)))
         end
 
         def from_json

lib/pact_broker/api/resources/environments.rb

@@ -27,11 +27,7 @@ def post_is_create?
         end
 
         def malformed_request?
-          if request.post?
-            invalid_json? || validation_errors_for_schema?(schema, params.merge(uuid: uuid))
-          else
-            false
-          end
+          super || (request.post? && validation_errors_for_schema?(schema, params.merge(uuid: uuid)))
         end
 
         def create_path

lib/pact_broker/api/resources/pact.rb

@@ -42,12 +42,9 @@ def is_conflict?
           potential_duplicate_pacticipants?(pact_params.pacticipant_names) || merge_conflict || disallowed_modification?
         end
 
+
         def malformed_request?
-          if request.patch? || request.put?
-            invalid_json? || contract_validation_errors?(Contracts::PutPactParamsContract.new(pact_params), pact_params)
-          else
-            false
-          end
+          super || ((request.patch? || request.really_put?) && contract_validation_errors?(Contracts::PutPactParamsContract.new(pact_params), pact_params))
         end
 
         def resource_exists?

lib/pact_broker/api/resources/pact_webhooks.rb

@@ -25,10 +25,7 @@ def resource_exists?
         end
 
         def malformed_request?
-          if request.post?
-            return invalid_json? || validation_errors?(webhook)
-          end
-          false
+          super || (request.post? && validation_errors?(webhook))
         end
 
         def validation_errors? webhook

lib/pact_broker/api/resources/pacticipant.rb

@@ -26,11 +26,7 @@ def known_methods
         end
 
         def malformed_request?
-          if request.patch? || request.put?
-            invalid_json? || validation_errors_for_schema?
-          else
-            false
-          end
+          super || ((request.patch? || request.really_put?) && validation_errors_for_schema?)
         end
 
         def from_json

lib/pact_broker/api/resources/pacticipant_webhooks.rb

@@ -27,10 +27,7 @@ def resource_exists?
         end
 
         def malformed_request?
-          if request.post?
-            return invalid_json? || webhook_validation_errors?(webhook)
-          end
-          false
+          super || (request.post? && webhook_validation_errors?(webhook))
         end
 
         def create_path

lib/pact_broker/api/resources/pacticipants.rb

@@ -23,10 +23,7 @@ def allowed_methods
         end
 
         def malformed_request?
-          if request.post?
-            return invalid_json? || validation_errors_for_schema?
-          end
-          false
+          super || (request.post? && validation_errors_for_schema?)
         end
 
         def post_is_create?

lib/pact_broker/api/resources/provider_pacts_for_verification.rb

@@ -20,12 +20,7 @@ def content_types_accepted
         end
 
         def malformed_request?
-          if (errors = query_schema.call(query)).any?
-            set_json_validation_error_messages(errors)
-            true
-          else
-            false
-          end
+          super || ((request.get? || request.post?) && schema_validation_errors?)
         end
 
         def process_post
@@ -96,6 +91,15 @@ def log_request
         def nested_query
           @nested_query ||= Rack::Utils.parse_nested_query(request.uri.query)
         end
+
+        def schema_validation_errors?
+          if (errors = query_schema.call(query)).any?
+            set_json_validation_error_messages(errors)
+            true
+          else
+            false
+          end
+        end
       end
     end
   end

lib/pact_broker/api/resources/publish_contracts.rb

@@ -23,11 +23,7 @@ def allowed_methods
         end
 
         def malformed_request?
-          if request.post?
-            invalid_json? || validation_errors_for_schema?
-          else
-            false
-          end
+          super || (request.post? && validation_errors_for_schema?)
         end
 
         def process_post

lib/pact_broker/api/resources/released_version.rb

@@ -8,11 +8,6 @@ module Resources
       class ReleasedVersion < BaseResource
         include PactBroker::Messages
 
-        def initialize
-          super
-          @currently_supported_param = params(default: {})[:currentlySupported]
-        end
-
         def content_types_provided
           [["application/hal+json", :to_json]]
         end
@@ -63,7 +58,14 @@ def policy_record
 
         private
 
-        attr_reader :currently_supported_param
+        # can't use ||= with a potentially nil value
+        def currently_supported_param
+          if defined?(@currently_deployed_param)
+            @currently_supported_param
+          else
+            @currently_supported_param = params(default: {})[:currentlySupported]
+          end
+        end
 
         def process_currently_supported_param
           if currently_supported_param == false

lib/pact_broker/api/resources/verifications.rb

@@ -34,15 +34,7 @@ def resource_exists?
         end
 
         def malformed_request?
-          if request.post?
-            return true if invalid_json?
-            errors = verification_service.errors(params)
-            if !errors.empty?
-              set_json_validation_error_messages(errors.messages)
-              return true
-            end
-          end
-          false
+          super || (request.post? && any_validation_errors?)
         end
 
         def create_path
@@ -91,6 +83,12 @@ def event_context
         def verification_params
           params(symbolize_names: false).merge("wip" => wip?, "pending" => pending?)
         end
+
+        def any_validation_errors?
+          errors = verification_service.errors(params)
+          set_json_validation_error_messages(errors.messages) if !errors.empty?
+          !errors.empty?
+        end
       end
     end
   end

lib/pact_broker/api/resources/version.rb

@@ -25,14 +25,6 @@ def resource_exists?
           !!version
         end
 
-        def malformed_request?
-          if request.put? && any_request_body?
-            invalid_json?
-          else
-            false
-          end
-        end
-
         def from_json
           if request.really_put?
             handle_request do

lib/pact_broker/api/resources/webhook.rb

@@ -27,10 +27,7 @@ def resource_exists?
         end
 
         def malformed_request?
-          if request.put?
-            return invalid_json? || webhook_validation_errors?(parsed_webhook, uuid)
-          end
-          false
+          super || (request.put? && webhook_validation_errors?(parsed_webhook, uuid))
         end
 
         def from_json

lib/pact_broker/api/resources/webhook_execution.rb

@@ -37,14 +37,16 @@ def resource_exists?
         end
 
         def malformed_request?
-          if request.post?
+          if super
+            true
+          elsif request.post?
             if uuid
               false
             else
               webhook_validation_errors?(webhook)
             end
           else
-            super
+            false
           end
         end