Skip to content

Commit

Permalink
Stop swallowing JSON parse errors (#67)
Browse files Browse the repository at this point in the history
* make safe_json_parse unsafe

* remove obsolete safe_json_parse method

* fix malformed json in specs
  • Loading branch information
cooperka authored May 17, 2024
1 parent b214e45 commit f50fe1e
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 30 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
/pkg/
/spec/reports/
/tmp/
/vendor/bundle/
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ In strict mode, passing empty destination arrays will always fail.

Bug reports and pull requests are welcome on GitHub at https://github.com/jetruby/apollo_upload_server-ruby. This project is intended to be a safe, welcoming space for collaboration, and contributors are expected to adhere to the [Contributor Covenant](http://contributor-covenant.org) code of conduct.

Tests can be run via `bundle exec rspec`.

## License

The gem is available as open source under the terms of the [MIT License](https://opensource.org/licenses/MIT).
Expand Down
10 changes: 2 additions & 8 deletions lib/apollo_upload_server/graphql_data_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ def initialize(strict_mode: false)
end

def call(params)
operations = safe_json_parse(params['operations'])
file_mapper = safe_json_parse(params['map'])
operations = JSON.parse(params['operations'])
file_mapper = JSON.parse(params['map'])

return nil if operations.nil? || file_mapper.nil?
if operations.is_a?(Hash)
Expand Down Expand Up @@ -62,12 +62,6 @@ def verify_array_index!(path, index, size)
raise OutOfBounds, "Path #{path.join('.')} maps to out-of-bounds index: #{index}"
end

def safe_json_parse(data)
JSON.parse(data)
rescue JSON::ParserError
nil
end

def get_parent_field(operations, splited_path)
# returns parent element of file field

Expand Down
44 changes: 22 additions & 22 deletions spec/apollo_upload_server/middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
require 'apollo_upload_server/middleware'

describe ApolloUploadServer::Middleware do
around do |example|
mode = described_class.strict_mode
example.run
described_class.strict_mode = mode
end

around do |example|
mode = described_class.strict_mode
example.run
described_class.strict_mode = mode
end

describe '#call' do
let(:app) do
Rack::Builder.new do
Expand All @@ -19,34 +19,34 @@

context "when CONTENT_TYPE is 'multipart/form-data'" do
subject do
Rack::MockRequest.new(app).post('/', { 'CONTENT_TYPE' => 'multipart/form-data', input: 'operations=foo&map=bar' })
Rack::MockRequest.new(app).post('/', { 'CONTENT_TYPE' => 'multipart/form-data', input: 'operations={}&map={}' })
end

it { expect(subject.status).to eq(200) }
end

context "when CONTENT_TYPE is not 'multipart/form-data'" do
subject do
Rack::MockRequest.new(app).post('/', { 'CONTENT_TYPE' => 'text/plain' })
Rack::MockRequest.new(app).post('/', { 'CONTENT_TYPE' => 'text/plain' })
end

it { expect(subject.status).to eq(200) }
end

context 'when configured to run in strict mode' do
before do
described_class.strict_mode = true
end

subject do
Rack::MockRequest.new(app).post('/', { 'CONTENT_TYPE' => 'multipart/form-data', input: 'operations=foo&map=bar' })
end

it 'propagates this setting to the data builder' do

context 'when configured to run in strict mode' do
before do
described_class.strict_mode = true
end

subject do
Rack::MockRequest.new(app).post('/', { 'CONTENT_TYPE' => 'multipart/form-data', input: 'operations={}&map={}' })
end

it 'propagates this setting to the data builder' do
expect(ApolloUploadServer::GraphQLDataBuilder).to receive(:new).with(strict_mode: true).and_call_original

subject
end
end
subject
end
end
end
end

0 comments on commit f50fe1e

Please sign in to comment.