Fixing file script filter

* The filter was filtering through all the filename instead of the extension. * Also including a migration to fix all the files that were misinterpreted.

Fixing file script filter
* The filter was filtering through all the filename instead of the extension. * Also including a migration to fix all the files that were misinterpreted.
Rodrigo Souto
1 parent 0db12ddd
Showing 3 changed files with 73 additions and 6 deletions Show diff stats
app/models/environment.rb
db/migrate/20110706171330_fix_misunderstood_script_filename.rb
test/unit/environment_test.rb
@@ -9,10 +9,10 @@ class Environment &lt; ActiveRecord::Base
  
   has_many :tasks, :dependent => :destroy, :as => 'target'
  
-  IDENTIFY_SCRIPTS = /(?:php[0-9s]?(\..*)?|[sp]htm[l]?(\..*)?|pl|py|cgi|rb)/
+  IDENTIFY_SCRIPTS = /(php[0-9s]?|[sp]htm[l]?|pl|py|cgi|rb)/
  
   def self.verify_filename(filename)
-    filename += '.txt' if filename =~ IDENTIFY_SCRIPTS
+    filename += '.txt' if File.extname(filename) =~ IDENTIFY_SCRIPTS
     filename
   end
  
@@ -0,0 +1,57 @@
+#FIXME Don't know why, but this xss_terminate and sanitize_tag_list calls here
+# from the migration fall on a loop and breaks the migration. Both them are
+# related to alias_method_chain, probably there is a problem with this kind of
+# alias on the migration level.
+class Article < ActiveRecord::Base
+  def sanitize_tag_list
+  end
+end
+
+module XssTerminate
+  module InstanceMethods
+    def sanitize_fields_with_white_list
+    end
+  end
+end
+
+#FIXME This after save calls the environment methods 'blocks' and
+# 'portal_community'. Both acts as not defined don't know why.
+class ArticleSweeper < ActiveRecord::Observer
+  def after_save(article)
+  end
+end
+
+class Environment < ActiveRecord::Base
+  def self.verify_filename(filename)
+    filename
+  end
+end
+
+class FixMisunderstoodScriptFilename < ActiveRecord::Migration
+  def self.up
+    Image.all.select { |i| !i.thumbnail? && File.extname(i.filename) == '.txt'}.map do |image|
+      image.thumbnails.destroy_all
+      image.filename = fixed_name(image)
+      image.save!
+      image.create_thumbnails
+    end
+
+    UploadedFile.all.select { |u| u.content_type != 'text/plain' && File.extname(u.filename) == '.txt' }.map do |uploaded_file|
+      uploaded_file.thumbnails.destroy_all
+      uploaded_file.filename = fixed_name(uploaded_file)
+      uploaded_file.save!
+      uploaded_file.create_thumbnails
+    end
+  end
+
+  def self.down
+    say "WARNING: cannot undo this migration"
+  end
+
+  class << self
+    def fixed_name(file)
+      file.filename.gsub('.txt', '')
+    end
+  end
+
+end
@@ -1124,15 +1124,25 @@ class EnvironmentTest &lt; Test::Unit::TestCase
   end
  
   should 'identify scripts with regex' do
-    scripts_extensions = %w[php php1 php4 phps php.bli cgi shtm phtm shtml phtml pl py rb]
-    name = 'uploaded_file'
+    scripts_extensions = %w[php php1 php4 phps cgi shtm phtm shtml phtml pl py rb]
     scripts_extensions.each do |extension|
-      assert_not_nil name+'.'+extension =~ Environment::IDENTIFY_SCRIPTS
+      assert_not_nil extension =~ Environment::IDENTIFY_SCRIPTS
     end
   end
  
+  should 'filter file as script only if it has the extension as a script extension' do
+    name = 'file_php_testing'
+    assert_equal name, Environment.verify_filename(name)
+
+    name += '.php'
+    assert_equal name+'.txt', Environment.verify_filename(name)
+
+    name += '.bli'
+    assert_equal name, Environment.verify_filename(name)
+  end
+
   should 'verify filename and append .txt if script' do
-    scripts_extensions = %w[php php1 php4 phps php.bli cgi shtm phtm shtml phtml pl py rb]
+    scripts_extensions = %w[php php1 php4 phps cgi shtm phtm shtml phtml pl py rb]
     name = 'uploaded_file'
     scripts_extensions.each do |extension|
       filename = name+'.'+extension
...	...	@@ -9,10 +9,10 @@ class Environment < ActiveRecord::Base
9	9
10	10	has_many :tasks, :dependent => :destroy, :as => 'target'
11	11
12		- IDENTIFY_SCRIPTS = /(?:php[0-9s]?(\..)?\|[sp]htm[l]?(\..)?\|pl\|py\|cgi\|rb)/
	12	+ IDENTIFY_SCRIPTS = /(php[0-9s]?\|[sp]htm[l]?\|pl\|py\|cgi\|rb)/
13	13
14	14	def self.verify_filename(filename)
15		- filename += '.txt' if filename =~ IDENTIFY_SCRIPTS
	15	+ filename += '.txt' if File.extname(filename) =~ IDENTIFY_SCRIPTS
16	16	filename
17	17	end
18	18
...	...
...	...	@@ -0,0 +1,57 @@
	1	+#FIXME Don't know why, but this xss_terminate and sanitize_tag_list calls here
	2	+# from the migration fall on a loop and breaks the migration. Both them are
	3	+# related to alias_method_chain, probably there is a problem with this kind of
	4	+# alias on the migration level.
	5	+class Article < ActiveRecord::Base
	6	+ def sanitize_tag_list
	7	+ end
	8	+end
	9	+
	10	+module XssTerminate
	11	+ module InstanceMethods
	12	+ def sanitize_fields_with_white_list
	13	+ end
	14	+ end
	15	+end
	16	+
	17	+#FIXME This after save calls the environment methods 'blocks' and
	18	+# 'portal_community'. Both acts as not defined don't know why.
	19	+class ArticleSweeper < ActiveRecord::Observer
	20	+ def after_save(article)
	21	+ end
	22	+end
	23	+
	24	+class Environment < ActiveRecord::Base
	25	+ def self.verify_filename(filename)
	26	+ filename
	27	+ end
	28	+end
	29	+
	30	+class FixMisunderstoodScriptFilename < ActiveRecord::Migration
	31	+ def self.up
	32	+ Image.all.select { \|i\| !i.thumbnail? && File.extname(i.filename) == '.txt'}.map do \|image\|
	33	+ image.thumbnails.destroy_all
	34	+ image.filename = fixed_name(image)
	35	+ image.save!
	36	+ image.create_thumbnails
	37	+ end
	38	+
	39	+ UploadedFile.all.select { \|u\| u.content_type != 'text/plain' && File.extname(u.filename) == '.txt' }.map do \|uploaded_file\|
	40	+ uploaded_file.thumbnails.destroy_all
	41	+ uploaded_file.filename = fixed_name(uploaded_file)
	42	+ uploaded_file.save!
	43	+ uploaded_file.create_thumbnails
	44	+ end
	45	+ end
	46	+
	47	+ def self.down
	48	+ say "WARNING: cannot undo this migration"
	49	+ end
	50	+
	51	+ class << self
	52	+ def fixed_name(file)
	53	+ file.filename.gsub('.txt', '')
	54	+ end
	55	+ end
	56	+
	57	+end
...	...
...	...	@@ -1124,15 +1124,25 @@ class EnvironmentTest < Test::Unit::TestCase
1124	1124	end
1125	1125
1126	1126	should 'identify scripts with regex' do
1127		- scripts_extensions = %w[php php1 php4 phps php.bli cgi shtm phtm shtml phtml pl py rb]
1128		- name = 'uploaded_file'
	1127	+ scripts_extensions = %w[php php1 php4 phps cgi shtm phtm shtml phtml pl py rb]
1129	1128	scripts_extensions.each do \|extension\|
1130		- assert_not_nil name+'.'+extension =~ Environment::IDENTIFY_SCRIPTS
	1129	+ assert_not_nil extension =~ Environment::IDENTIFY_SCRIPTS
1131	1130	end
1132	1131	end
1133	1132
	1133	+ should 'filter file as script only if it has the extension as a script extension' do
	1134	+ name = 'file_php_testing'
	1135	+ assert_equal name, Environment.verify_filename(name)
	1136	+
	1137	+ name += '.php'
	1138	+ assert_equal name+'.txt', Environment.verify_filename(name)
	1139	+
	1140	+ name += '.bli'
	1141	+ assert_equal name, Environment.verify_filename(name)
	1142	+ end
	1143	+
1134	1144	should 'verify filename and append .txt if script' do
1135		- scripts_extensions = %w[php php1 php4 phps php.bli cgi shtm phtm shtml phtml pl py rb]
	1145	+ scripts_extensions = %w[php php1 php4 phps cgi shtm phtm shtml phtml pl py rb]
1136	1146	name = 'uploaded_file'
1137	1147	scripts_extensions.each do \|extension\|
1138	1148	filename = name+'.'+extension
...	...