fix infinite parent folder loop

test plan:
* try to update the name for a root folder of a course
* should give an error, instead of catastrophically
 freezing your server

fixes #CNVS-14868

Change-Id: If0a1c305f3103f3bb7e17ff6ceead7db2e2173de
Reviewed-on: https://gerrit.instructure.com/39512
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Clare Strong <clare@instructure.com>
This commit is contained in:
James Williams 2014-08-19 06:49:14 -06:00
parent 113cdf510c
commit f06611aa6e
2 changed files with 29 additions and 9 deletions

View File

@ -58,8 +58,22 @@ class Folder < ActiveRecord::Base
before_save :infer_hidden_state
validates_presence_of :context_id, :context_type
validates_length_of :name, :maximum => maximum_string_length
validate :protect_root_folder_name, :if => :name_changed?
validate :reject_recursive_folder_structures, on: :update
def protect_root_folder_name
if self.parent_folder_id.blank? && self.name != Folder.root_folder_name_for_context(context)
if self.new_record?
root_folder = Folder.root_folders(context).first
self.parent_folder_id = root_folder.id
return true
else
errors.add(:name, t("errors.invalid_root_folder_name", "Root folder name cannot be changed"))
return false
end
end
end
def reject_recursive_folder_structures
return true if !self.parent_folder_id_changed?
seen_folders = Set.new([self])
@ -121,11 +135,6 @@ class Folder < ActiveRecord::Base
def default_values
self.last_unlock_at = self.unlock_at if self.unlock_at
self.last_lock_at = self.lock_at if self.lock_at
if self.parent_folder_id.blank? && ![ROOT_FOLDER_NAME, MY_FILES_FOLDER_NAME, 'files'].include?(self.name)
root_folder = Folder.root_folders(context).first
self.parent_folder_id = root_folder.id
end
end
def infer_hidden_state
@ -255,15 +264,18 @@ class Folder < ActiveRecord::Base
!self.parent_folder_id
end
def self.root_folders(context)
def self.root_folder_name_for_context(context)
if context.is_a? Course
name = ROOT_FOLDER_NAME
ROOT_FOLDER_NAME
elsif context.is_a? User
name = MY_FILES_FOLDER_NAME
MY_FILES_FOLDER_NAME
else
name = "files"
"files"
end
end
def self.root_folders(context)
name = root_folder_name_for_context(context)
root_folders = []
# something that doesn't have folders?!
return root_folders unless context.respond_to?(:folders)

View File

@ -61,6 +61,14 @@ describe Folder do
f1.errors.detect { |e| e.first.to_s == 'parent_folder_id' }.should be_present
end
it "should not allow root folders to have their names changed" do
f1 = Folder.root_folders(@course).first
f1.reload
f1.update_attributes(:name => "something")
f1.save.should == false
f1.errors.detect { |e| e.first.to_s == 'name' }.should be_present
end
it "files without an explicit folder_id should be inferred" do
f = @course.folders.create!(:name => "unfiled")
a = f.active_file_attachments.build