Skip to content

Commit de7e7d3

Browse files
committed
Fix 628 CSV parsing errors (#937)
* Added test and potential fix for #628. * Add handling for CSV parsing errors. (cherry picked from commit 9306ec0)
1 parent d46a080 commit de7e7d3

3 files changed

Lines changed: 56 additions & 10 deletions

File tree

lib/jsonapi/request_parser.rb

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -210,23 +210,28 @@ def check_include(resource_klass, include_parts)
210210
end
211211
end
212212

213-
def parse_include_directives(include)
214-
return if include.nil?
213+
def parse_include_directives(raw_include)
214+
return unless raw_include
215215

216216
unless JSONAPI.configuration.allow_include
217217
fail JSONAPI::Exceptions::ParametersNotAllowed.new([:include])
218218
end
219219

220-
included_resources = CSV.parse_line(include)
221-
return if included_resources.nil?
220+
included_resources = []
221+
begin
222+
included_resources += CSV.parse_line(raw_include)
223+
rescue CSV::MalformedCSVError
224+
fail JSONAPI::Exceptions::InvalidInclude.new(format_key(@resource_klass._type), raw_include)
225+
end
226+
227+
return if included_resources.empty?
222228

223-
include = []
224-
included_resources.each do |included_resource|
229+
result = included_resources.map do |included_resource|
225230
check_include(@resource_klass, included_resource.partition('.'))
226-
include.push(unformat_key(included_resource).to_s)
231+
unformat_key(included_resource).to_s
227232
end
228233

229-
@include_directives = JSONAPI::IncludeDirectives.new(@resource_klass, include)
234+
@include_directives = JSONAPI::IncludeDirectives.new(@resource_klass, result)
230235
end
231236

232237
def parse_filters(filters)
@@ -265,7 +270,15 @@ def parse_sort_criteria(sort_criteria)
265270
fail JSONAPI::Exceptions::ParametersNotAllowed.new([:sort])
266271
end
267272

268-
@sort_criteria = CSV.parse_line(URI.unescape(sort_criteria)).collect do |sort|
273+
sorts = []
274+
begin
275+
raw = URI.unescape(sort_criteria)
276+
sorts += CSV.parse_line(raw)
277+
rescue CSV::MalformedCSVError
278+
fail JSONAPI::Exceptions::InvalidSortCriteria.new(format_key(@resource_klass._type), raw)
279+
end
280+
281+
@sort_criteria = sorts.collect do |sort|
269282
if sort.start_with?('-')
270283
sort_criteria = { field: unformat_key(sort[1..-1]).to_s }
271284
sort_criteria[:direction] = :desc

lib/jsonapi/resource.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -820,7 +820,11 @@ def is_filter_relationship?(filter)
820820
def verify_filter(filter, raw, context = nil)
821821
filter_values = []
822822
if raw.present?
823-
filter_values += raw.is_a?(String) ? CSV.parse_line(raw) : [raw]
823+
begin
824+
filter_values += raw.is_a?(String) ? CSV.parse_line(raw) : [raw]
825+
rescue CSV::MalformedCSVError
826+
filter_values << raw
827+
end
824828
end
825829

826830
strategy = _allowed_filters.fetch(filter, Hash.new)[:verify]

test/integration/requests/request_test.rb

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ def test_get_underscored_key
4545
JSONAPI.configuration = original_config
4646
end
4747

48+
def test_filter_with_value_containing_double_quote
49+
original_config = JSONAPI.configuration.dup
50+
JSONAPI.configuration.json_key_format = :underscored_key
51+
get '/iso_currencies?filter[country_name]=%22'
52+
assert_jsonapi_response 200
53+
ensure
54+
JSONAPI.configuration = original_config
55+
end
56+
4857
def test_get_underscored_key_filtered
4958
original_config = JSONAPI.configuration.dup
5059
JSONAPI.configuration.json_key_format = :underscored_key
@@ -1063,6 +1072,26 @@ def test_sort_parameter_not_allowed
10631072
JSONAPI.configuration.allow_sort = true
10641073
end
10651074

1075+
def test_sort_parameter_quoted
1076+
get '/api/v2/books?sort=%22title%22', headers: { 'Accept' => JSONAPI::MEDIA_TYPE }
1077+
assert_jsonapi_response 200
1078+
end
1079+
1080+
def test_sort_parameter_openquoted
1081+
get '/api/v2/books?sort=%22title', headers: { 'Accept' => JSONAPI::MEDIA_TYPE }
1082+
assert_jsonapi_response 400
1083+
end
1084+
1085+
def test_include_parameter_quoted
1086+
get '/api/v2/posts?include=%22author%22', headers: { 'Accept' => JSONAPI::MEDIA_TYPE }
1087+
assert_jsonapi_response 200
1088+
end
1089+
1090+
def test_include_parameter_openquoted
1091+
get '/api/v2/posts?include=%22author', headers: { 'Accept' => JSONAPI::MEDIA_TYPE }
1092+
assert_jsonapi_response 400
1093+
end
1094+
10661095
def test_getting_different_resources_when_sti
10671096
assert_cacheable_jsonapi_get '/vehicles'
10681097
types = json_response['data'].map{|r| r['type']}.sort

0 commit comments

Comments
 (0)