Skip to content

Stop swallowing JSON parse errors #67

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

Merged
merged 3 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Common path for local gems to be stored

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

Copy link
Contributor Author

@cooperka cooperka May 16, 2024

Choose a reason for hiding this comment

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

Honestly not sure what changed here, tabs and line endings seem unaltered 🤷

View the diff with ?w=1 in URL params to show actual changes.

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