Skip to content

Commit

Permalink
Handle gracefully only JSON parsing errors
Browse files Browse the repository at this point in the history
  • Loading branch information
alxberardi committed Dec 19, 2016
1 parent b17359c commit 90863d2
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 25 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ This changelog adheres to [Keep a CHANGELOG](http://keepachangelog.com/).
- Updates README for usage with Rails

### Fixed
- Handles parsing exceptions gracefully
- Handles JSON parsing exceptions gracefully

## [0.2.0] - 2016-12-13
### Added
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ The arguments for `config.register_parser` are:
* a parser for the parameters
* an unparser to convert parameters back to the encoded format

The parser and unparser must be objects that respond to `call` and accept the parameters as an argument (e.g. procs or lambdas).
The parser and unparser must be objects that respond to `call` and accept the parameters as an argument (e.g. procs or lambdas).
The parser should handle parsing exceptions gracefully by returning the arguments.
This ensures that sensitive data scanning and masking is applied on the raw parameters.

## Development

Expand Down
22 changes: 16 additions & 6 deletions lib/sensitive_data_filter/middleware/parameter_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,10 @@ def can_parse?(content_type)

def parse(params)
@parse.call params
rescue
params
end

def unparse(params)
@unparse.call params
rescue
params
end

NULL_PARSER = new('', ->(params) { params }, ->(params) { params })
Expand All @@ -44,9 +40,23 @@ def unparse(params)
->(params) { Rack::Utils.parse_query(params) },
->(params) { Rack::Utils.build_query(params) }),
new('json', # e.g.: 'application/json'
->(params) { JSON.parse(params) },
->(params) { JSON.unparse(params) })
->(params) { JsonParser.new.parse(params) },
->(params) { JsonParser.new.unparse(params) })
].freeze

class JsonParser
def parse(params)
JSON.parse(params)
rescue JSON::ParserError
params
end

def unparse(params)
JSON.unparse(params)
rescue JSON::GeneratorError
params
end
end
end
end
end
26 changes: 9 additions & 17 deletions spec/sensitive_data/middleware/parameter_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@
specify { expect(parser).not_to be null_parser }
specify { expect(parser.parse(parameters)).to eq 'test' => true }
specify { expect(parser.unparse('test' => true)).to eq parameters }

context 'when parsing raises exceptions' do
specify { expect{parser.parse('not json')}.not_to raise_error }
specify { expect(parser.parse('not json')).to eq 'not json' }

let(:nan) { 0.0/0 }
specify { expect{parser.unparse(nan)}.not_to raise_error }
specify { expect(parser.unparse(nan)).to be nan }
end
end

describe 'a custom parser' do
Expand Down Expand Up @@ -68,23 +77,6 @@
end
end

context 'when parsing raises exceptions' do
let(:content_type) { 'application/test' }

before do
parser_class.register_parser(
'test',
->(_params) { fail 'Parsing Error' },
->(_params) { fail 'Parsing Error' }
)
end

specify { expect{parser.parse('test')}.not_to raise_error }
specify { expect(parser.parse('test')).to eq 'test' }
specify { expect{parser.unparse('test')}.not_to raise_error }
specify { expect(parser.unparse('test')).to eq 'test' }
end

context 'when the content type is nil' do
let(:content_type) { nil }
specify { expect(parser).to be null_parser }
Expand Down

0 comments on commit 90863d2

Please sign in to comment.