Skip to content

Commit

Permalink
feat: Avoid unexpected transformations for parameters
Browse files Browse the repository at this point in the history
Restify checked every param if it responds to to_param, and calls it,
before passing values to Addressable::Template. Since Rails patches
to_param into anything, that resulted in accepting virtually anything
somehow into params.

For example, any Array was encoded a slash-delimited string of the
individual values ([1,2] -> "1/2"), which not only could result in
confusing things accidentially passed as params, but also made it
impossible to pass a parameter multiple times (a: [1, 2] -> "a=1&a=2").

This commit takes the basic type detection from Addressable::Template
and tries to only apply to_param, which addressable does not support at
all, for non-basic types. Therefore, arrays and hash, should behave
similar to when passed directly to Addressable::Template, but it will
still be possible to e.g. pass an ActiveRecord model as a parameter,
using #to_param.

This makes passing standard and Rails-style argument lists possible:

    expand(p: [1, 2])       -> "/?p=1&p=2"
    expand('p[]': [1, 2])   -> "/?p%5B%5D=1&p%5B%5D=2"

Fixes #44
  • Loading branch information
jgraichen committed Jan 17, 2025
1 parent a33193a commit 1399989
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 2 deletions.
26 changes: 24 additions & 2 deletions lib/restify/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,31 @@ def convert(params)
end
end

def convert_param(value)
return value.to_param.to_s if value.respond_to?(:to_param)
def convert_param(value, nesting: true)
# Convert parameters into values acceptable in a
# Addressable::Template, with some support for #to_param, but not
# for basic types.
if value.is_a?(Numeric) || value.is_a?(Symbol) || value == true || value == false
return value
end

# Handle Arrays first to *not* call #to_params on them, which will
# concatenation any Array to "a/b/c". Instead, we want to check
# one level of basic types only.
if value.is_a?(Array)
return nesting ? value.map {|val| convert_param(val) } : value
end

# Ignore Hashes
return value if value.is_a?(Hash)

# Handle Rails' #to_param for non-basic types
if value.respond_to?(:to_param)
return value.to_param
end

# Otherwise, pass raw value to Addressable::Template and let is
# explode.
value
end

Expand Down
31 changes: 31 additions & 0 deletions spec/restify/relation_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require 'spec_helper'
require 'active_support'

describe Restify::Relation do
subject(:relation) { described_class.new context, pattern }
Expand Down Expand Up @@ -34,6 +35,12 @@ def to_param
it { expect(expanded.to_s).to eq 'http://test.host/resource/42' }
end

context 'with array parameter' do
let(:params) { {id: [1, 2]} }

it { expect(expanded.to_s).to eq 'http://test.host/resource/1,2' }
end

context 'with unknown additional query parameter' do
let(:pattern) { '/resource{?a,b}' }
let(:params) { {a: 1, b: 2, c: 3} }
Expand All @@ -52,5 +59,29 @@ def to_param

it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=42' }
end

context 'with additional array parameter' do
let(:params) { {id: 5, abc: [1, 2]} }

it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=1&abc=2' }
end

context 'with additional array parameter with objects' do
let(:params) { {id: 5, abc: [1, 2, cls_to_param.new]} }

it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=1&abc=2&abc=42' }
end

context 'with additional nested array parameter' do
let(:params) { {id: 5, abc: [1, 2, [1]]} }

it { expect { expanded }.to raise_exception TypeError, /Can't convert Array into String./ }
end

context 'with additional hash parameter' do
let(:params) { {id: 5, abc: {a: 1, b: 2}} }

it { expect { expanded }.to raise_exception TypeError, /Can't convert Hash into String./ }
end
end
end

0 comments on commit 1399989

Please sign in to comment.