Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Hash#to_param confused by empty hash values #13892

Closed
jhawthorn opened this Issue · 5 comments

3 participants

@jhawthorn

When to_param is used with empty hash values, it generates incorrect results.

> {}.to_param
""

Seems reasonable, but once nesting is introduced, this adds an extra ampersand at the start of the result.

> {c: 3, d: {}}.to_param
"&b%5Bc%5D=3"
> CGI::unescape _
"&b[c]=3"

With another level of nesting, the params aren't sorted lexicographically (which they should be according to the documentation).

> {a: 1, b: {c: 3, d: {}}}.to_param
"&b%5Bc%5D=3&a=1"
> CGI::unescape _
"&b[c]=3&a=1"

This is because "&b[c]=3" < "a=1"

Expected Result

I'm uncertain how exactly to_param should behave in these cases.

It could either behave like nil

> CGI::unescape({a: 1, b: {c: 3, d: nil}}.to_param)
"a=1&b[c]=3&b[d]="

or behave as it does now (not adding any params), but not adding the extra ampersand

"a=1&b[c]=3"

If I could get some clarification on what the result should be I'd be happy to submit a pull request.

EDIT: Seems empty arrays also behave this way

@sandipransing

@jhawthorn +1

  1. Order is not correct

    CGI::unescape({a: 1, b: {c: 3, d: {}}}.to_param)
    => "&b[c]=3&a=1" 
    
  2. If value is nil or "" it shows up

    CGI::unescape({p: 12, b: {c: 3, e: nil, f: "" }}.to_param)
    => "b[c]=3&b[e]=&b[f]=&p=12" 
    
  3. If value is empty hash {} then it's not showing up

    CGI::unescape({b: {c: 3, k: {}, f: "" }}.to_param)
    => "&b[c]=3&b[f]=" 
    

Expectation:

CGI::unescape({a: 1, b: {c: 3, d: {}}}.to_param)
 => "&a=1&b[c]=3" 
CGI::unescape({p: 12, b: {c: 3, e: nil, f: "" }}.to_param)
 => "b[c]=3&b[e]=&b[f]=&p=12" 
CGI::unescape({b: {c: 3, k: {}, f: "" }}.to_param)
 => "&b[c]=3&b[f]=&b[k]=" 
@rafaelfranca
Owner

Closed by #13909

@jhawthorn

Thanks, but this is still broken for arrays.

{a: [], b: 3}.to_param #=> "&b=3"
@rafaelfranca
Owner

Fixed at c82dbc6

@jhawthorn

Thanks so much @rafaelfranca :heart:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.