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

FI-2728: Prevent Test Runner from Crashing When Validator Response Contains Bad Characters #518

Merged
merged 9 commits into from
Aug 27, 2024
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Style/BlockComments:

Style/Documentation:
Enabled: false

Style/FrozenStringLiteralComment:
Enabled: false

Expand Down
28 changes: 14 additions & 14 deletions lib/inferno/dsl/fhir_resource_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'] && 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
Expand All @@ -295,14 +294,15 @@ 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
# Sanitize the response body by removing non-printable/control characters
Copy link
Collaborator

Choose a reason for hiding this comment

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

Get rid of the comment and make a method called remove_invalid_characters or something like that.

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#{sanitized_body}")
raise Inferno::Exceptions::ErrorInValidatorException,
'Validator response was an unexpected format. '\
'Review Messages tab or validator service logs for more information.'
end
end

Expand Down
19 changes: 10 additions & 9 deletions lib/inferno/dsl/fhir_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -214,14 +214,15 @@ 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
# 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#{sanitized_body}")
raise Inferno::Exceptions::ErrorInValidatorException,
'Validator response was an unexpected format. '\
'Review Messages tab or validator service logs for more information.'
end
end

Expand Down
19 changes: 9 additions & 10 deletions lib/inferno/jobs/invoke_validator_session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions spec/inferno/dsl/fhir_resource_validation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: "<html><body>Internal Server Error: content#{0.chr} with non-printable#{1.chr} characters</body></html>"
)

expect do
validator.resource_is_valid?(resource2, profile_url, runnable)
end.to raise_error(Inferno::Exceptions::ErrorInValidatorException)

msg = runnable.messages.first[:message]
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

describe '.cli_context' do
Expand Down
18 changes: 18 additions & 0 deletions spec/inferno/dsl/fhir_validation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: "<html><body>Internal Server Error: content#{0.chr} with non-printable#{1.chr} characters</body></html>"
)

expect do
validator.resource_is_valid?(resource, profile_url, runnable)
end.to raise_error(Inferno::Exceptions::ErrorInValidatorException)

msg = runnable.messages.first[:message]
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

describe '.find_validator' do
Expand Down
Loading