From 75540d0617944968063b507661a8fe119bc010c4 Mon Sep 17 00:00:00 2001 From: Vanessa Fotso Date: Thu, 15 Aug 2024 22:38:03 -0400 Subject: [PATCH 1/8] Improve JSON parsing error handling for validator responses Signed-off-by: Vanessa Fotso --- lib/inferno/dsl/fhir_resource_validation.rb | 25 +++++++++----------- lib/inferno/dsl/fhir_validation.rb | 16 ++++++------- lib/inferno/jobs/invoke_validator_session.rb | 19 +++++++-------- 3 files changed, 27 insertions(+), 33 deletions(-) diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb index 1421c14b7..0ed14b674 100644 --- a/lib/inferno/dsl/fhir_resource_validation.rb +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -277,16 +277,15 @@ def call_validator(resource, profile_url) end # @private - def operation_outcome_from_hl7_wrapped_response(response) - res = JSON.parse(response) - if res['sessionId'] != @session_id - validator_session_repo.save(test_suite_id:, validator_session_id: res['sessionId'], + def operation_outcome_from_hl7_wrapped_response(response_hash) + if response_hash['sessionId'] != @session_id + validator_session_repo.save(test_suite_id:, validator_session_id: response_hash['sessionId'], validator_name: name.to_s, suite_options: requirements) - @session_id = res['sessionId'] + @session_id = response_hash['sessionId'] end # assume for now that one resource -> one request - issues = res['outcomes'][0]['issues']&.map do |i| + issues = response_hash['outcomes'][0]['issues']&.map do |i| { severity: i['level'].downcase, expression: i['location'], details: { text: i['message'] } } end # this is circuitous, ideally we would map this response directly to message_hashes @@ -295,14 +294,12 @@ def operation_outcome_from_hl7_wrapped_response(response) # @private def operation_outcome_from_validator_response(response, runnable) - if response.body.start_with? '{' - operation_outcome_from_hl7_wrapped_response(response.body) - else - runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{response.body}") - raise Inferno::Exceptions::ErrorInValidatorException, - 'Validator response was an unexpected format. ' \ - 'Review Messages tab or validator service logs for more information.' - end + operation_outcome_from_hl7_wrapped_response(JSON.parse(response.body)) + rescue JSON::ParserError + runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{response.body}") + raise Inferno::Exceptions::ErrorInValidatorException, + 'Validator response was an unexpected format. '\ + 'Review Messages tab or validator service logs for more information.' end end diff --git a/lib/inferno/dsl/fhir_validation.rb b/lib/inferno/dsl/fhir_validation.rb index 24c8ce0ca..cb1229413 100644 --- a/lib/inferno/dsl/fhir_validation.rb +++ b/lib/inferno/dsl/fhir_validation.rb @@ -124,7 +124,7 @@ def resource_is_valid?(resource, profile_url, runnable) runnable.add_message('error', e.message) raise Inferno::Exceptions::ErrorInValidatorException, "Unable to connect to validator at #{url}." end - outcome = operation_outcome_from_validator_response(response.body, runnable) + outcome = operation_outcome_from_validator_response(response, runnable) message_hashes = message_hashes_from_outcome(outcome, resource, profile_url) @@ -214,14 +214,12 @@ def call_validator(resource, profile_url) # @private def operation_outcome_from_validator_response(response, runnable) - if response.start_with? '{' - FHIR::OperationOutcome.new(JSON.parse(response)) - else - runnable.add_message('error', "Validator Response:\n#{response}") - raise Inferno::Exceptions::ErrorInValidatorException, - 'Validator response was an unexpected format. '\ - 'Review Messages tab or validator service logs for more information.' - end + FHIR::OperationOutcome.new(JSON.parse(response.body)) + rescue JSON::ParserError + runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{response.body}") + raise Inferno::Exceptions::ErrorInValidatorException, + 'Validator response was an unexpected format. '\ + 'Review Messages tab or validator service logs for more information.' end end diff --git a/lib/inferno/jobs/invoke_validator_session.rb b/lib/inferno/jobs/invoke_validator_session.rb index 691594073..0a25da8b6 100644 --- a/lib/inferno/jobs/invoke_validator_session.rb +++ b/lib/inferno/jobs/invoke_validator_session.rb @@ -8,16 +8,15 @@ def perform(suite_id, validator_name, validator_index) suite = Inferno::Repositories::TestSuites.new.find suite_id validator = suite.fhir_validators[validator_name.to_sym][validator_index] response_body = validator.validate(FHIR::Patient.new, 'http://hl7.org/fhir/StructureDefinition/Patient') - if response_body.start_with? '{' - res = JSON.parse(response_body) - session_id = res['sessionId'] - session_repo = Inferno::Repositories::ValidatorSessions.new - session_repo.save(test_suite_id: suite_id, validator_session_id: session_id, - validator_name:, suite_options: validator.requirements) - validator.session_id = session_id - else - Inferno::Application['logger'].error("InvokeValidatorSession - error from validator: #{response_body}") - end + res = JSON.parse(response_body) + session_id = res['sessionId'] + session_repo = Inferno::Repositories::ValidatorSessions.new + session_repo.save(test_suite_id: suite_id, validator_session_id: session_id, + validator_name:, suite_options: validator.requirements) + validator.session_id = session_id + rescue JSON::ParserError + Inferno::Application['logger'] + .error("InvokeValidatorSession - error unexpected response format from validator: #{response_body}") end end end From d29d43aaefe6a43e90a27cae591ee9f0d43b5204 Mon Sep 17 00:00:00 2001 From: Vanessa Fotso <46642178+vanessuniq@users.noreply.github.com> Date: Fri, 23 Aug 2024 10:34:55 -0400 Subject: [PATCH 2/8] prevent storing nil validator session ids Co-authored-by: Dylan Hall --- lib/inferno/dsl/fhir_resource_validation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb index 0ed14b674..f1c75a86b 100644 --- a/lib/inferno/dsl/fhir_resource_validation.rb +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -278,7 +278,7 @@ def call_validator(resource, profile_url) # @private def operation_outcome_from_hl7_wrapped_response(response_hash) - if response_hash['sessionId'] != @session_id + if response_hash['sessionId'] && response_hash['sessionId'] != @session_id validator_session_repo.save(test_suite_id:, validator_session_id: response_hash['sessionId'], validator_name: name.to_s, suite_options: requirements) @session_id = response_hash['sessionId'] From 78a0a07f39947ef5e3c77766efc3957020e2bd36 Mon Sep 17 00:00:00 2001 From: Vanessa Fotso Date: Sun, 25 Aug 2024 15:21:28 -0400 Subject: [PATCH 3/8] handled presence of non printable char in the validator response Signed-off-by: Vanessa Fotso --- lib/inferno/dsl/fhir_resource_validation.rb | 7 +++++-- lib/inferno/dsl/fhir_validation.rb | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb index f1c75a86b..a63e4b3c3 100644 --- a/lib/inferno/dsl/fhir_resource_validation.rb +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -294,9 +294,12 @@ def operation_outcome_from_hl7_wrapped_response(response_hash) # @private def operation_outcome_from_validator_response(response, runnable) - operation_outcome_from_hl7_wrapped_response(JSON.parse(response.body)) + # Sanitize the response body by removing non-printable/control characters + sanitized_body = response.body.gsub(/[^[:print:]\r\n]+/, '') + + operation_outcome_from_hl7_wrapped_response(JSON.parse(sanitized_body)) rescue JSON::ParserError - runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{response.body}") + runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{sanitized_body}") raise Inferno::Exceptions::ErrorInValidatorException, 'Validator response was an unexpected format. '\ 'Review Messages tab or validator service logs for more information.' diff --git a/lib/inferno/dsl/fhir_validation.rb b/lib/inferno/dsl/fhir_validation.rb index cb1229413..23e1f7b84 100644 --- a/lib/inferno/dsl/fhir_validation.rb +++ b/lib/inferno/dsl/fhir_validation.rb @@ -214,9 +214,12 @@ def call_validator(resource, profile_url) # @private def operation_outcome_from_validator_response(response, runnable) - FHIR::OperationOutcome.new(JSON.parse(response.body)) + # Sanitize the response body by removing non-printable/control characters + sanitized_body = response.body.gsub(/[^[:print:]\r\n]+/, '') + + FHIR::OperationOutcome.new(JSON.parse(sanitized_body)) rescue JSON::ParserError - runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{response.body}") + runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{sanitized_body}") raise Inferno::Exceptions::ErrorInValidatorException, 'Validator response was an unexpected format. '\ 'Review Messages tab or validator service logs for more information.' From 63ec527884dd7a4ba30cbfa825c86c522f5c491b Mon Sep 17 00:00:00 2001 From: Vanessa Fotso Date: Mon, 26 Aug 2024 09:52:07 -0400 Subject: [PATCH 4/8] updated spec Signed-off-by: Vanessa Fotso --- .../dsl/fhir_resource_validation_spec.rb | 18 ++++++++++++++++++ spec/inferno/dsl/fhir_validation_spec.rb | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/spec/inferno/dsl/fhir_resource_validation_spec.rb b/spec/inferno/dsl/fhir_resource_validation_spec.rb index a617cb396..41199dcc0 100644 --- a/spec/inferno/dsl/fhir_resource_validation_spec.rb +++ b/spec/inferno/dsl/fhir_resource_validation_spec.rb @@ -170,6 +170,24 @@ validator.resource_is_valid?(resource2, profile_url, runnable) end.to raise_error(Inferno::Exceptions::ErrorInValidatorException) end + + it 'removes non-printable characters from the response' do + stub_request(:post, "#{validation_url}/validate") + .with(body: wrapped_resource_string) + .to_return( + status: 500, + body: "Internal Server Error: content#{0.chr} with non-printable#{1.chr} characters" + ) + + expect do + validator.resource_is_valid?(resource2, profile_url, runnable) + end.to raise_error(Inferno::Exceptions::ErrorInValidatorException) + + msg = runnable.messages.first[:message] + expect(msg).not_to include(0.chr) + expect(msg).not_to include(1.chr) + expect(msg).to match(/Internal Server Error: content with non-printable/) + end end describe '.cli_context' do diff --git a/spec/inferno/dsl/fhir_validation_spec.rb b/spec/inferno/dsl/fhir_validation_spec.rb index 0458e9d17..354b7d4f5 100644 --- a/spec/inferno/dsl/fhir_validation_spec.rb +++ b/spec/inferno/dsl/fhir_validation_spec.rb @@ -176,6 +176,24 @@ expect(validator.resource_is_valid?(resource, profile_url, runnable)).to be(true) end + + it 'removes non-printable characters from the response' do + stub_request(:post, "#{validation_url}/validate?profile=#{profile_url}") + .with(body: resource_string) + .to_return( + status: 500, + body: "Internal Server Error: content#{0.chr} with non-printable#{1.chr} characters" + ) + + expect do + validator.resource_is_valid?(resource, profile_url, runnable) + end.to raise_error(Inferno::Exceptions::ErrorInValidatorException) + + msg = runnable.messages.first[:message] + expect(msg).not_to include(0.chr) + expect(msg).not_to include(1.chr) + expect(msg).to match(/Internal Server Error: content with non-printable/) + end end describe '.find_validator' do From eb476c6a820c7fd77018458d2d1d2fb1a7a7dcad Mon Sep 17 00:00:00 2001 From: Vanessa Fotso Date: Mon, 26 Aug 2024 10:17:19 -0400 Subject: [PATCH 5/8] replaced deprecated RSpec/FilePath cop with RSpec/FilePathFormat Signed-off-by: Vanessa Fotso --- .rubocop.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 7c88f8b36..52715c67b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -32,7 +32,7 @@ Style/BlockComments: Style/Documentation: Enabled: false - + Style/FrozenStringLiteralComment: Enabled: false @@ -81,7 +81,7 @@ RSpec/DescribeClass: RSpec/ExampleLength: Enabled: false -RSpec/FilePath: +RSpec/SpecFilePathFormat: CustomTransform: OAuthCredentials: oauth_credentials From 6716a84f743250180d8fe29a4bdbc11c2a5519ee Mon Sep 17 00:00:00 2001 From: Vanessa Fotso Date: Mon, 26 Aug 2024 10:18:48 -0400 Subject: [PATCH 6/8] linting Signed-off-by: Vanessa Fotso --- spec/inferno/dsl/fhir_resource_validation_spec.rb | 6 +++--- spec/inferno/dsl/fhir_validation_spec.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/inferno/dsl/fhir_resource_validation_spec.rb b/spec/inferno/dsl/fhir_resource_validation_spec.rb index 41199dcc0..216ca11cb 100644 --- a/spec/inferno/dsl/fhir_resource_validation_spec.rb +++ b/spec/inferno/dsl/fhir_resource_validation_spec.rb @@ -177,15 +177,15 @@ .to_return( status: 500, body: "Internal Server Error: content#{0.chr} with non-printable#{1.chr} characters" - ) + ) expect do validator.resource_is_valid?(resource2, profile_url, runnable) end.to raise_error(Inferno::Exceptions::ErrorInValidatorException) msg = runnable.messages.first[:message] - expect(msg).not_to include(0.chr) - expect(msg).not_to include(1.chr) + expect(msg).to_not include(0.chr) + expect(msg).to_not include(1.chr) expect(msg).to match(/Internal Server Error: content with non-printable/) end end diff --git a/spec/inferno/dsl/fhir_validation_spec.rb b/spec/inferno/dsl/fhir_validation_spec.rb index 354b7d4f5..94c5a3784 100644 --- a/spec/inferno/dsl/fhir_validation_spec.rb +++ b/spec/inferno/dsl/fhir_validation_spec.rb @@ -183,15 +183,15 @@ .to_return( status: 500, body: "Internal Server Error: content#{0.chr} with non-printable#{1.chr} characters" - ) + ) expect do validator.resource_is_valid?(resource, profile_url, runnable) end.to raise_error(Inferno::Exceptions::ErrorInValidatorException) msg = runnable.messages.first[:message] - expect(msg).not_to include(0.chr) - expect(msg).not_to include(1.chr) + expect(msg).to_not include(0.chr) + expect(msg).to_not include(1.chr) expect(msg).to match(/Internal Server Error: content with non-printable/) end end From e765ead7317044f7a33feabac386743affcf450f Mon Sep 17 00:00:00 2001 From: Vanessa Fotso Date: Mon, 26 Aug 2024 10:23:10 -0400 Subject: [PATCH 7/8] linting Signed-off-by: Vanessa Fotso --- .rubocop.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 52715c67b..571621e64 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -81,7 +81,7 @@ RSpec/DescribeClass: RSpec/ExampleLength: Enabled: false -RSpec/SpecFilePathFormat: +RSpec/FilePath: CustomTransform: OAuthCredentials: oauth_credentials From a894c0c2fa9a3d22812a098ac86f565ecd002c6d Mon Sep 17 00:00:00 2001 From: Vanessa Fotso Date: Mon, 26 Aug 2024 14:32:39 -0400 Subject: [PATCH 8/8] Extract character sanitization logic into remove_invalid_characters method Signed-off-by: Vanessa Fotso --- lib/inferno/dsl/fhir_resource_validation.rb | 10 +++++++--- lib/inferno/dsl/fhir_validation.rb | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/inferno/dsl/fhir_resource_validation.rb b/lib/inferno/dsl/fhir_resource_validation.rb index a63e4b3c3..540e50d5d 100644 --- a/lib/inferno/dsl/fhir_resource_validation.rb +++ b/lib/inferno/dsl/fhir_resource_validation.rb @@ -292,16 +292,20 @@ def operation_outcome_from_hl7_wrapped_response(response_hash) FHIR::OperationOutcome.new(issue: issues) end + # @private + def remove_invalid_characters(string) + string.gsub(/[^[:print:]\r\n]+/, '') + end + # @private def operation_outcome_from_validator_response(response, runnable) - # Sanitize the response body by removing non-printable/control characters - sanitized_body = response.body.gsub(/[^[:print:]\r\n]+/, '') + sanitized_body = remove_invalid_characters(response.body) operation_outcome_from_hl7_wrapped_response(JSON.parse(sanitized_body)) rescue JSON::ParserError runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{sanitized_body}") raise Inferno::Exceptions::ErrorInValidatorException, - 'Validator response was an unexpected format. '\ + 'Validator response was an unexpected format. ' \ 'Review Messages tab or validator service logs for more information.' end end diff --git a/lib/inferno/dsl/fhir_validation.rb b/lib/inferno/dsl/fhir_validation.rb index 23e1f7b84..3a21e1b3b 100644 --- a/lib/inferno/dsl/fhir_validation.rb +++ b/lib/inferno/dsl/fhir_validation.rb @@ -212,16 +212,20 @@ def call_validator(resource, profile_url) ).post('validate', resource.source_contents) end + # @private + def remove_invalid_characters(string) + string.gsub(/[^[:print:]\r\n]+/, '') + end + # @private def operation_outcome_from_validator_response(response, runnable) - # Sanitize the response body by removing non-printable/control characters - sanitized_body = response.body.gsub(/[^[:print:]\r\n]+/, '') + sanitized_body = remove_invalid_characters(response.body) FHIR::OperationOutcome.new(JSON.parse(sanitized_body)) rescue JSON::ParserError runnable.add_message('error', "Validator Response: HTTP #{response.status}\n#{sanitized_body}") raise Inferno::Exceptions::ErrorInValidatorException, - 'Validator response was an unexpected format. '\ + 'Validator response was an unexpected format. ' \ 'Review Messages tab or validator service logs for more information.' end end