Hello lovely folks,
first and foremost thanks for your excellent work on this project practically the entire ruby webdev eco system relies on 💚 🎉 🚀
Problem
I've come here today to propose a change in the parsing behavior of the query string param[].
There is a spec asserting the current behavior so I assume it's intentional
Rack ::Utils . parse_nested_query ( "foo[]" ) .
must_equal "foo" => [ nil ]
I believe/hope the more correct test would be:
Rack ::Utils . parse_nested_query ( "foo[]" ) .
must_equal "foo" => [ ]
I base this on three things:
we otherwise have no way to indicate an empty array and an array with nil doesn't seem particularly useful (or intuitive) vs. an empty array
the introduction was 12 years ago ( 6e330bd ) and might be a side effect of the implementation due to the splitting behavior exhibited here:
unless qs . nil? || qs . empty?
( qs || '' ) . split ( d ? ( COMMON_SEP [ d ] || /[#{ d } ] */n ) : DEFAULT_SEP ) . each do |p |
k , v = p . split ( '=' , 2 ) . map! { |s | unescape ( s ) }
normalize_params ( params , k , v , param_depth_limit )
end
the popular HTTP client library faraday (which her and many projects rely upon) changed it's way of serializing empty arrays to the query string above #800 correct handling parameter value empty array lostisland/faraday#801
Especially due to the last point this creates a sort of dissonance in the Ruby eco system where I send an empty array from the one side and it arrives as [nil] on the server side. Of course, the fix might also be on the faraday side.
I have not yet checked the HTTP specs or anything about this to see what's "officially correct"
To have complete traceability, I came here via ruby-grape/grape#2079 :)
Implementation
I'm not sure if it's the complete implementation but it seems to be easily fixable in
elsif after == "[]"
params [ k ] ||= [ ]
raise ParameterTypeError , "expected Array (got #{ params [ k ] . class . name } ) for param `#{ k } '" unless params [ k ] . is_a? ( Array )
params [ k ] << v
all it needs is unless v.nil?:
elsif after == "[]"
params [ k ] ||= [ ]
raise ParameterTypeError , "expected Array (got #{ params [ k ] . class . name } ) for param `#{ k } '" unless params [ k ] . is_a? ( Array )
params [ k ] << v unless v . nil?
All tests (with exception of the one mentioned above) pass with this change.
(I have the code, I found the old spec while implementing this/my spec was elsewhere)
Happy to make a PR should it be wanted!
Thanks for all your work! 🎉 🚀
Hello lovely folks,
first and foremost thanks for your excellent work on this project practically the entire ruby webdev eco system relies on 💚 🎉 🚀
Problem
I've come here today to propose a change in the parsing behavior of the query string
param[].There is a spec asserting the current behavior so I assume it's intentional
rack/test/spec_utils.rb
Lines 164 to 165 in 649c72b
I believe/hope the more correct test would be:
I base this on three things:
nildoesn't seem particularly useful (or intuitive) vs. an empty arrayrack/lib/rack/query_parser.rb
Lines 67 to 72 in 649c72b
herand many projects rely upon) changed it's way of serializing empty arrays to the query string above #800 correct handling parameter value empty array lostisland/faraday#801Especially due to the last point this creates a sort of dissonance in the Ruby eco system where I send an empty array from the one side and it arrives as
[nil]on the server side. Of course, the fix might also be on the faraday side.I have not yet checked the HTTP specs or anything about this to see what's "officially correct"
To have complete traceability, I came here via ruby-grape/grape#2079 :)
Implementation
I'm not sure if it's the complete implementation but it seems to be easily fixable in
rack/lib/rack/query_parser.rb
Lines 102 to 105 in 649c72b
all it needs is
unless v.nil?:All tests (with exception of the one mentioned above) pass with this change.
(I have the code, I found the old spec while implementing this/my spec was elsewhere)
Happy to make a PR should it be wanted!
Thanks for all your work! 🎉 🚀