Bug #6240

[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 about 4 years ago. Updated over 3 years ago.

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

History

#1 Updated by Tom Clegg about 4 years ago

  • Project changed from Arvados Private to Arvados
  • Description updated (diff)
  • Category set to API
  • Target version set to Bug Triage

Noticed during #6074.

#2 Updated by Tom Clegg about 4 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.

#3 Updated by Brett Smith about 4 years ago

  • Target version changed from Bug Triage to Arvados Future Sprints

#4 Updated by Joshua Randall over 3 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?

Also available in: Atom PDF