From 9d3ace1fdba783134eb5557a9b28f8132df552de Mon Sep 17 00:00:00 2001
From: Peter Amstutz <peter.amstutz@curii.com>
Date: Thu, 4 May 2023 19:09:05 -0400
Subject: [PATCH] 20470: select_for_klass checks for bogus prefixed fields

Update comments

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>
---
 services/api/app/controllers/application_controller.rb | 10 +++++-----
 .../controllers/arvados/v1/containers_controller.rb    |  6 ++++++
 .../api/app/controllers/arvados/v1/users_controller.rb |  6 ++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 1704eb9132..b191550240 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -234,7 +234,7 @@ class ApplicationController < ActionController::Base
     # given class.
     sel.map do |column|
       sp = column.split(".")
-      if sp.length == 2 && sp[0] == model_class.table_name
+      if sp.length == 2 && sp[0] == model_class.table_name && model_class.selectable_attributes.include?(sp[1])
         sp[1]
       elsif model_class.selectable_attributes.include? column
         column
@@ -501,8 +501,8 @@ class ApplicationController < ActionController::Base
     # This is a little hacky but sometimes the fields the user wants
     # to selecting on are unrelated to the object being loaded here,
     # for example groups#contents, so filter the fields that will be
-    # used in find_objects_for_index and then reset it below.  In some
-    # cases, code that modifies the @select list needs to set
+    # used in find_objects_for_index and then reset afterwards.  In
+    # some cases, code that modifies the @select list needs to set
     # @preserve_select.
     @preserve_select = @select
     @select = select_for_klass(@select, self.model_class, false)
@@ -599,10 +599,10 @@ class ApplicationController < ActionController::Base
       :self_link => "",
       :offset => @offset,
       :limit => @limit,
-      :items => @objects.as_api_response(nil, {select: select_for_klass(@select, model_class)})
+      :items => @objects.as_api_response(nil, {select: @select})
     }
     if @extra_included
-      list[:included] = @extra_included.as_api_response(nil, {select: select_for_klass(@select, model_class)})
+      list[:included] = @extra_included.as_api_response(nil, {select: @select})
     end
     case params[:count]
     when nil, '', 'exact'
diff --git a/services/api/app/controllers/arvados/v1/containers_controller.rb b/services/api/app/controllers/arvados/v1/containers_controller.rb
index 507f9dcf1a..acb61c6e60 100644
--- a/services/api/app/controllers/arvados/v1/containers_controller.rb
+++ b/services/api/app/controllers/arvados/v1/containers_controller.rb
@@ -50,6 +50,12 @@ class Arvados::V1::ContainersController < ApplicationController
     if action_name == 'lock' || action_name == 'unlock'
       # Avoid loading more fields than we need
       @objects = @objects.select(:id, :uuid, :state, :priority, :auth_uuid, :locked_by_uuid, :lock_count)
+      # This gets called from within find_object_by_uuid.
+      # find_object_by_uuid stores the original value of @select in
+      # @preserve_select, edits the value of @select, calls
+      # find_objects_for_index, then restores @select from the value
+      # of @preserve_select.  So if we want our updated value of
+      # @select here to stick, we have to set @preserve_select.
       @select = @preserve_select = %w(uuid state priority auth_uuid locked_by_uuid)
     elsif action_name == 'update_priority'
       # We're going to reload(lock: true) in the handler, which will
diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index e0573a51c9..ded86aa66d 100644
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -282,6 +282,12 @@ class Arvados::V1::UsersController < ApplicationController
       end
       @filters += [['is_active', '=', true]]
     end
+    # This gets called from within find_object_by_uuid.
+    # find_object_by_uuid stores the original value of @select in
+    # @preserve_select, edits the value of @select, calls
+    # find_objects_for_index, then restores @select from the value
+    # of @preserve_select.  So if we want our updated value of
+    # @select here to stick, we have to set @preserve_select.
     @preserve_select = @select
     super
   end
-- 
2.30.2