On 10/03/2011 07:41 PM, Chris Alfonso wrote:
the :conditions are parameterized, but the :order clauses need to be
processed
to guard against sql injection. There are a couple of approaches to do this.
The option I selected was to make sure the order column specified by the user
actually exists on the object.
---
src/app/controllers/application_controller.rb | 8 ++++++++
src/app/controllers/deployments_controller.rb | 4 ++--
.../controllers/hardware_profiles_controller.rb | 6 +++---
src/app/controllers/instances_controller.rb | 2 +-
src/app/controllers/pool_families_controller.rb | 2 +-
src/app/controllers/pools_controller.rb | 10 +++++-----
src/app/controllers/roles_controller.rb | 2 +-
7 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/src/app/controllers/application_controller.rb
b/src/app/controllers/application_controller.rb
index b2d7453..92574ae 100644
--- a/src/app/controllers/application_controller.rb
+++ b/src/app/controllers/application_controller.rb
@@ -241,4 +241,12 @@ class ApplicationController< ActionController::Base
raise "Tab '#{tab}' doesn't exist"
end
end
+
+ def sort_column(model, default="name")
+ model.column_names.include?(params[:order_field]) ? params[:order_field] : default
+ end
+
+ def sort_direction
+ %w[asc desc].include?(params[:order_dir]) ? params[:order_dir] : "asc"
+ end
end
diff --git a/src/app/controllers/deployments_controller.rb
b/src/app/controllers/deployments_controller.rb
index 4c0389b..d727aff 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -285,7 +285,7 @@ class DeploymentsController< ApplicationController
private
def load_deployments
@deployments = Deployment.paginate(:page => params[:page] || 1,
- :order => (params[:order_field] || 'name') +' '+
(params[:order_dir] || 'asc')
+ :order => (sort_column(Deployment) +' '+ sort_direction)
)
@deployments_header = [
{ :name => '', :sortable => false },
@@ -301,7 +301,7 @@ class DeploymentsController< ApplicationController
@pools = Pool.list_for_user(current_user, Privilege::CREATE, :target_type =>
Deployment)
@deployments = Deployment.all(:include => :owner,
:conditions => {:pool_id => @pools},
- :order => (params[:order_field] || 'name')
+' '+ (params[:order_dir] || 'asc')
+ :order => (sort_column(Deployment) +' '+
sort_direction)
)
end
diff --git a/src/app/controllers/hardware_profiles_controller.rb
b/src/app/controllers/hardware_profiles_controller.rb
index 8df760f..90e9c06 100644
--- a/src/app/controllers/hardware_profiles_controller.rb
+++ b/src/app/controllers/hardware_profiles_controller.rb
@@ -202,10 +202,10 @@ class HardwareProfilesController< ApplicationController
end
def load_hardware_profiles
- sort_field = params[:order_field].blank? ? "name" : params[:order_field]
- sort_order = params[:order_dir].to_s == "desc" ? "desc" :
"asc"
+ sort_order = sort_direction
+ sort_field = sort_column(HardwareProfile)
if sort_field == "name"
- @hardware_profiles = HardwareProfile.list_for_user(current_user, Privilege::VIEW,
:order => "hardware_profiles.name #{sort_order}", :conditions =>
['provider_id IS NULL', {}])
+ @hardware_profiles = HardwareProfile.list_for_user(current_user, Privilege::VIEW,
:order => "hardware_profiles.name #{sort_direction}", :conditions =>
['provider_id IS NULL', {}])
else
@hardware_profiles = HardwareProfile.list_for_user(current_user, Privilege::VIEW,
:conditions => ['provider_id IS NULL', {}])
if sort_order == "asc"
diff --git a/src/app/controllers/instances_controller.rb
b/src/app/controllers/instances_controller.rb
index 617f358..a449350 100644
--- a/src/app/controllers/instances_controller.rb
+++ b/src/app/controllers/instances_controller.rb
@@ -208,7 +208,7 @@ class InstancesController< ApplicationController
conditions[:deployment_id] = params[:deployment_id] unless
params[:deployment_id].blank?
@instances = Instance.all(:include => [:owner],
:conditions => conditions,
- :order => (params[:order_field] || 'name')
+' '+ (params[:order_dir] || 'asc')
+ :order => (sort_column(Instance) +' '+
sort_direction)
)
end
end
diff --git a/src/app/controllers/pool_families_controller.rb
b/src/app/controllers/pool_families_controller.rb
index 0a7cc9a..eea79ee 100644
--- a/src/app/controllers/pool_families_controller.rb
+++ b/src/app/controllers/pool_families_controller.rb
@@ -159,6 +159,6 @@ class PoolFamiliesController< ApplicationController
def load_pool_families
@pool_families = PoolFamily.list_for_user(current_user, Privilege::VIEW).paginate(
:page => params[:page] || 1,
- :order => (params[:order_field] || 'name') + ' ' +
(params[:order_dir] || 'asc'))
+ :order => (sort_column(PoolFamily) + ' ' + sort_direction))
end
end
diff --git a/src/app/controllers/pools_controller.rb
b/src/app/controllers/pools_controller.rb
index 043af1d..ffbb256 100644
--- a/src/app/controllers/pools_controller.rb
+++ b/src/app/controllers/pools_controller.rb
@@ -48,18 +48,18 @@ class PoolsController< ApplicationController
@details_tab = @tabs.find {|t| t[:id] == details_tab_name} ||
@tabs.first[:name].downcase
case @details_tab[:id]
when 'pools'
- @pools = Pool.list(params[:order_field],params[:order_dir])
+ @pools = Pool.list(sort_column(Pool), sort_direction)
when 'instances'
- @instances = Instance.list(params[:order_field],params[:order_dir])
+ @instances = Instance.list(sort_column(Instance), sort_direction)
@hide_stopped = @viewstate&& @viewstate.state['hide_stopped']
== 'true'
if @hide_stopped
@instances.delete_if { |i| i.state == Instance::STATE_STOPPED }
end
when 'deployments'
- @deployments = Deployment.list(params[:order_field],params[:order_dir])
+ @deployments = Deployment.list(sort_column(Deployment), sort_direction)
end
else
- @pools = Pool.list(params[:order_field],params[:order_dir])
+ @pools = Pool.list(sort_column(Pool), sort_direction)
end
statistics
respond_to do |format|
@@ -268,7 +268,7 @@ class PoolsController< ApplicationController
def load_pools
@pools = Pool.paginate(:include => [ :quota, :pool_family ],
:page => params[:page] || 1,
- :order => (params[:order_field] || 'name') +' '+
(params[:order_dir] || 'asc')
+ :order => (sort_column(Pool)+' '+ sort_direction)
)
end
diff --git a/src/app/controllers/roles_controller.rb
b/src/app/controllers/roles_controller.rb
index 9f023bd..64735eb 100644
--- a/src/app/controllers/roles_controller.rb
+++ b/src/app/controllers/roles_controller.rb
@@ -117,7 +117,7 @@ class RolesController< ApplicationController
def load_roles
@roles = Role.paginate(:page => params[:page] || 1,
- :order => (params[:order_field] || 'name') +' '+
(params[:order_dir] || 'asc')
+ :order => (sort_column(Role)+' '+ (sort_direction))
)
end
ACK
as we discussed yesterday: we will probably have to enhance this once we
will want sort by joined table column. But this approach works fine for now.
Jan