[PATCH/foreman 1/1] fixes #1313 ensure all host names are lower cased

Florian_Koch · November 30, 2011, 6:24pm

Signed-off-by: Florian Koch <florian.koch1981@googlemail.com>

···

--- app/controllers/application_controller.rb | 3 +++ app/models/host.rb | 1 + ...130180548_ensure_all_hostnames_are_lowercase.rb | 13 +++++++++++++ 3 files changed, 17 insertions(+), 0 deletions(-) create mode 100644 db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 0783d91…ed6ecd0 100644
— a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -115,6 +115,9 @@ class ApplicationController < ActionController::Base
def find_by_name
not_found and return if (id = params[:id]).blank?

#1313 ensure all host names are lower cased
params[:id].downcase!
obj = controller_name.singularize

determine if we are searching for a numerical id or plain name

cond = “find_by_” + ((id =~ /^\d+$/ && id=id.to_i) ? “id” : “name”)
diff --git a/app/models/host.rb b/app/models/host.rb
index cf0c0fd…132c4d1 100644
— a/app/models/host.rb
+++ b/app/models/host.rb
@@ -117,6 +117,7 @@ class Host < Puppet::Rails::Host

validates_uniqueness_of :name
validates_presence_of :name, :environment_id
validates_format_of :name , :with => /^[-a-z\d.]+$/ , :message => “letters must be lowercase”
if SETTINGS[:unattended]

handles all orchestration of smart proxies.

include Foreman::Renderer
diff --git a/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb b/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb
new file mode 100644
index 0000000…cd1b87e
— /dev/null
+++ b/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb
@@ -0,0 +1,13 @@
+class EnsureAllHostnamesAreLowercase < ActiveRecord::Migration
def self.up

   execute "UPDATE hosts SET name = LOWER(name)"

end
def self.down

   raise ActiveRecord::IrreversibleMigration

end
+end
–
1.7.6.4

ohadlevy · December 13, 2011, 12:42pm

>
> Signed-off-by: Florian Koch <florian.koch1981@googlemail.com>
> —
> app/controllers/application_controller.rb | 3 +++
> app/models/host.rb | 1 +
> …130180548_ensure_all_hostnames_are_lowercase.rb | 13 +++++++++++++
> 3 files changed, 17 insertions(+), 0 deletions(-)
> create mode 100644 db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb
>
> diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
> index 0783d91…ed6ecd0 100644
> — a/app/controllers/application_controller.rb
> +++ b/app/controllers/application_controller.rb
> @@ -115,6 +115,9 @@ class ApplicationController < ActionController::Base
> def find_by_name
> not_found and return if (id = params[:id]).blank?
>
> + #1313 ensure all host names are lower cased
> + params[:id].downcase!
> +
that would effect all controllers which uses this, while it might not
be a big deal (as I cant think of any id which might be case
sensative) it might be better to do something like this in the host
controller

def find_by_name
params[:id].downcase! if params[:id].present?
super
end
> obj = controller_name.singularize
> # determine if we are searching for a numerical id or plain name
> cond = "find_by_" + ((id =~ /^\d+$/ && id=id.to_i) ? "id" : "name")
> diff --git a/app/models/host.rb b/app/models/host.rb
> index cf0c0fd…132c4d1 100644
> — a/app/models/host.rb
> +++ b/app/models/host.rb
> @@ -117,6 +117,7 @@ class Host < Puppet::Rails::Host
>
> validates_uniqueness_of :name
> validates_presence_of :name, :environment_id
> + validates_format_of :name , :with => /^[-a-z\d.]+$/ , :message => "letters must be lowercase"

are you sure thats a regexp that catches all possible hostnames? I
would play it safe and simply check if name == name.downcase

> if SETTINGS[:unattended]
> # handles all orchestration of smart proxies.
> include Foreman::Renderer
> diff --git a/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb b/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb
> new file mode 100644
> index 0000000…cd1b87e
> — /dev/null
> +++ b/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb
> @@ -0,0 +1,13 @@
> +class EnsureAllHostnamesAreLowercase < ActiveRecord::Migration
> + def self.up
> +
> + execute "UPDATE hosts SET name = LOWER(name)"
> +
I'm guessing this works on all sqlite/mysql/pg ?

> + end
> +
> + def self.down
> +
> + raise ActiveRecord::IrreversibleMigration
> +
> + end
> +end
> –
> 1.7.6.4
>

would you mind ensuring that all tests pass too? I'm quite sure we
have some fixtures (sample) data which is using upper cases.

Thanks!
Ohad

···

On Wed, Nov 30, 2011 at 8:24 PM, Florian Koch wrote:

Florian_Koch · December 21, 2011, 9:38am

> On Wed, Nov 30, 2011 at 8:24 PM, Florian Koch
>
>
>
>
>
>
>
>
> > Signed-off-by: Florian Koch <florian.koch1...@googlemail.com>
> > —
> > app/controllers/application_controller.rb | 3 +++
> > app/models/host.rb | 1 +
> > …130180548_ensure_all_hostnames_are_lowercase.rb | 13 +++++++++++++
> > 3 files changed, 17 insertions(+), 0 deletions(-)
> > create mode 100644 db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb
>
> > diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
> > index 0783d91…ed6ecd0 100644
> > — a/app/controllers/application_controller.rb
> > +++ b/app/controllers/application_controller.rb
> > @@ -115,6 +115,9 @@ class ApplicationController < ActionController::Base
> > def find_by_name
> > not_found and return if (id = params[:id]).blank?
>
> > + #1313 ensure all host names are lower cased
> > + params[:id].downcase!
> > +
>
> that would effect all controllers which uses this, while it might not
> be a big deal (as I cant think of any id which might be case
> sensative) it might be better to do something like this in the host
> controller
>
> def find_by_name
> params[:id].downcase! if params[:id].present?
> super
> end

hm i think the name should always lowercased ? but i think for this
fix we can define a new function

> > obj = controller_name.singularize
> > # determine if we are searching for a numerical id or plain name
> > cond = "find_by_" + ((id =~ /^\d+$/ && id=id.to_i) ? "id" : "name")
> > diff --git a/app/models/host.rb b/app/models/host.rb
> > index cf0c0fd…132c4d1 100644
> > — a/app/models/host.rb
> > +++ b/app/models/host.rb
> > @@ -117,6 +117,7 @@ class Host < Puppet::Rails::Host
>
> > validates_uniqueness_of :name
> > validates_presence_of :name, :environment_id
> > + validates_format_of :name , :with => /^[-a-z\d.]+$/ , :message => "letters must be lowercase"
>
> are you sure thats a regexp that catches all possible hostnames? I
> would play it safe and simply check if name == name.downcase
>
> > if SETTINGS[:unattended]
> > # handles all orchestration of smart proxies.
> > include Foreman::Renderer
> > diff --git a/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb b/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb
> > new file mode 100644
> > index 0000000…cd1b87e
> > — /dev/null
> > +++ b/db/migrate/20111130180548_ensure_all_hostnames_are_lowercase.rb
> > @@ -0,0 +1,13 @@
> > +class EnsureAllHostnamesAreLowercase < ActiveRecord::Migration
> > + def self.up
> > +
> > + execute "UPDATE hosts SET name = LOWER(name)"
> > +
>
> I'm guessing this works on all sqlite/mysql/pg ?

it's plain sql, so i should work on any sql compatible RDBMS

> > + end
> > +
> > + def self.down
> > +
> > + raise ActiveRecord::IrreversibleMigration
> > +
> > + end
> > +end
> > –
> > 1.7.6.4
>
> would you mind ensuring that all tests pass too? I'm quite sure we
> have some fixtures (sample) data which is using upper cases.

no haven't , try this now

···

On Dec 13, 1:42 pm, Ohad Levy wrote: > wrote: > Thanks! > Ohad