From 8d63678d1406c5518d437709af0fde717c0248d7 Mon Sep 17 00:00:00 2001 From: Philippe Huibonhoa Date: Wed, 21 Sep 2011 21:00:46 -0700 Subject: [PATCH 1/2] Fixed issue in file store where it could create a filename that was too long for the file system. (https://github.com/rails/rails/issues/3072) --- .../lib/active_support/cache/file_store.rb | 17 ++++++++--------- activesupport/test/caching_test.rb | 9 +++++++++ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index f7c01948b4..ab2382d98c 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -13,6 +13,7 @@ module ActiveSupport attr_reader :cache_path DIR_FORMATTER = "%03X" + FILENAME_MAX_SIZE = 230 # max filename size on file system is 255, minus room for timestamp and random characters appended by Tempfile (used by atomic write) def initialize(cache_path, options = nil) super(options) @@ -129,15 +130,13 @@ module ActiveSupport hash, dir_1 = hash.divmod(0x1000) dir_2 = hash.modulo(0x1000) fname_paths = [] - # Make sure file name is < 255 characters so it doesn't exceed file system limits. - if fname.size <= 255 - fname_paths << fname - else - while fname.size <= 255 - fname_path << fname[0, 255] - fname = fname[255, -1] - end - end + + # Make sure file name doesn't exceed file system limits. + begin + fname_paths << fname[0...FILENAME_MAX_SIZE] + fname = fname[FILENAME_MAX_SIZE..-1] + end until fname.blank? + File.join(cache_path, DIR_FORMATTER % dir_1, DIR_FORMATTER % dir_2, *fname_paths) end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 6bb13ec9b8..6b113594cc 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -557,6 +557,15 @@ class FileStoreTest < ActiveSupport::TestCase key = @cache_with_pathname.send(:key_file_path, "views/index?id=1") assert_equal "views/index?id=1", @cache_with_pathname.send(:file_path_key, key) end + + # Because file systems have a maximum filename size, filenames > max size should be split in to directories + # If filename is 'AAAAB', where max size is 4, the returned path should be AAAA/B + def test_key_transformation_max_filename_size + key = "#{'A' * ActiveSupport::Cache::FileStore::FILENAME_MAX_SIZE}B" + path = @cache.send(:key_file_path, key) + assert path.split('/').all? { |dir_name| dir_name.size <= ActiveSupport::Cache::FileStore::FILENAME_MAX_SIZE} + assert_equal 'B', File.basename(path) + end end class MemoryStoreTest < ActiveSupport::TestCase From a0352a425f4995f7f1e1035290fe59c93ac0d24f Mon Sep 17 00:00:00 2001 From: Philippe Huibonhoa Date: Wed, 21 Sep 2011 21:03:59 -0700 Subject: [PATCH 2/2] =?UTF-8?q?Updated=20existing=20test=20that=20fails=20?= =?UTF-8?q?with=20this=20fix=20(8d63678d1406c5518d437709af0fde717c0248d7).?= =?UTF-8?q?=20=20Before=20the=20fix=20the=20test=20was=20giving=20a=20fals?= =?UTF-8?q?e=20positive,=20because=20file=5Fstore.key=5Ffile=5Fpath=20woul?= =?UTF-8?q?d=20return=20an=20empty=20filename=20(i.e.=20test/tmp=5Fcache/4?= =?UTF-8?q?D0/F4D=20rather=20than=20test/tmp=5Fcache/4D0/F4D/xxxx=E2=80=A6?= =?UTF-8?q?).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even though the fix referenced above divides the filename into directories to prevent it from being too long, it seems that 1000 characters will always raise an error, so reducing the key size to 900). --- activesupport/test/caching_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 6b113594cc..b692a41312 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -357,7 +357,7 @@ module CacheStoreBehavior def test_really_long_keys key = "" - 1000.times{key << "x"} + 900.times{key << "x"} assert_equal true, @cache.write(key, "bar") assert_equal "bar", @cache.read(key) assert_equal "bar", @cache.fetch(key)