Actions
Bug #6240
open[API] "order" params should be rejected or used (not silently ignored) even when they reference columns missing from the "select" param
Status:
New
Priority:
Normal
Assigned To:
-
Category:
API
Target version:
-
Start date:
Due date:
% Done:
0%
Estimated time:
Story points:
0.5
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
Updated by Tom Clegg over 9 years ago
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.
Updated by Brett Smith over 9 years ago
- Target version changed from Bug Triage to Arvados Future Sprints
Updated by Joshua Randall almost 9 years ago
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?
Updated by Ward Vandewege over 3 years ago
- Target version deleted (
Arvados Future Sprints)
Actions