Merge pull request #4911 from Floppy/master

Reduce FILENAME_MAX_SIZE in ActiveSupport::Cache::FileStore

2 files changed, 11 insertions, 1 deletions

diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb
index b7712a4a15..8e6a3bc5a8 100644
--- a/activesupport/lib/active_support/cache/file_store.rb
+++ b/activesupport/lib/active_support/cache/file_store.rb
@@ -13,7 +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)
+      FILENAME_MAX_SIZE = 228 # max filename size on file system is 255, minus room for timestamp and random characters appended by Tempfile (used by atomic write)
       EXCLUDED_DIRS = ['.', '..'].freeze
 
       def initialize(cache_path, options = nil)
diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb
index ba027f1ff0..4371a02934 100644
--- a/activesupport/test/caching_test.rb
+++ b/activesupport/test/caching_test.rb
@@ -579,6 +579,16 @@ class FileStoreTest < ActiveSupport::TestCase
     assert_equal "views/index?id=1", @cache_with_pathname.send(:file_path_key, key)
   end
 
+  # Test that generated cache keys are short enough to have Tempfile stuff added to them and 
+  # remain valid
+  def test_filename_max_size
+    key = "#{'A' * ActiveSupport::Cache::FileStore::FILENAME_MAX_SIZE}"
+    path = @cache.send(:key_file_path, key)
+    Dir::Tmpname.create(path) do |tmpname, n, opts|
+      assert (File.basename(tmpname+'.lock').length <= 255), "Temp filename too long: #{File.basename(tmpname+'.lock').length}"
+    end
+  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