Bug #6240
open
[API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" param
Added by Tom Clegg almost 10 years ago.
Updated about 1 year ago.
Release relationship:
Auto
Description
Works:
tomclegg@shell.4xphq:~$ arv -s collection list --order 'created_at desc' --limit 2
4xphq-4zz18-dlgfut054qerwrz
4xphq-4zz18-cyfxpfhi4hfdf5u
tomclegg@shell.4xphq:~$ arv -s collection list --order 'created_at asc' --limit 2
4xphq-4zz18-0h10cbbqzuqx2ij
4xphq-4zz18-1nksjinjkpy3sja
tomclegg@shell.4xphq:~$ arv -s collection list --select '["uuid","created_at"]' --order 'created_at desc' --limit 2
4xphq-4zz18-dlgfut054qerwrz
4xphq-4zz18-cyfxpfhi4hfdf5u
tomclegg@shell.4xphq:~$ arv -s collection list --select '["uuid","created_at"]' --order 'created_at asc' --limit 2
4xphq-4zz18-0h10cbbqzuqx2ij
4xphq-4zz18-1nksjinjkpy3sja
Silently ignores given order:
tomclegg@shell.4xphq:~$ arv -s collection list --select '["uuid"]' --order 'created_at desc' --limit 2
4xphq-4zz18-004t28r3og3bodi
4xphq-4zz18-00jz1ybalavvh5y
tomclegg@shell.4xphq:~$ arv -s collection list --select '["uuid"]' --order 'created_at asc' --limit 2
4xphq-4zz18-004t28r3og3bodi
4xphq-4zz18-00jz1ybalavvh5y
- Project changed from 35 to Arvados
- Description updated (diff)
- Category set to API
- Target version set to Bug Triage
The easy fix (reject instead of ignoring) looks something like this, in source:services/api/lib/load_param.rb
if @select
# Any ordering columns must be selected when doing select,
# otherwise it is an SQL error, so filter out invaliding orderings.
- @orders.select! { |o|
+ @orders.each { |o|
# match select column against order array entry
- @select.select { |s| /^#{table_name}.#{s}( (asc|desc))?$/.match o }.any?
+ if not @select.select { |s| /^#{table_name}.#{s}( (asc|desc))?$/.match o }.any?
+ raise ArgumentError.new("Cannot order on '#{s}' when column excluded by select param")
+ end
}
end
IMO it would be vastly preferable to add the given column to the list of columns to select (without adding it to the list of attributes to return to the client). That might involve changing more than 2 lines of code, though.
- Target version changed from Bug Triage to Arvados Future Sprints
I just got caught by this. Definitely was not expecting the selected columns to have any bearing on the requested order. Not really understanding why this is not a simple thing to do... must be some difficulties with the Ruby ORM?
- Target version deleted (
Arvados Future Sprints)
- Target version set to Future
Also available in: Atom
PDF