New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce FILENAME_MAX_SIZE in ActiveSupport::Cache::FileStore #4911

Merged
merged 1 commit into from Mar 31, 2012

Conversation

Projects
None yet
4 participants
@Floppy
Contributor

Floppy commented Feb 6, 2012

In order that temp filenames generated from it will fit in 255 chars. See #4907

Change FILENAME_MAX_SIZE in FileStore to 228.
In order that temp filenames generated from it will fit in 255 chars. See #4907
@tenderlove

This comment has been minimized.

Member

tenderlove commented Feb 6, 2012

Seems good, but I'm a little annoyed because this value is OS (and possibly filesystem) specific. I'm annoyed that Ruby doesn't provide an API to pathconf so that we can determine the correct length. Without that value, we're just going to keep chasing numbers until this value is set to the lowest common denominator. :(

@Floppy

This comment has been minimized.

Contributor

Floppy commented Feb 6, 2012

I agree; it does feel crufty. Am open to a better solution if available...

@kalabiyau

This comment has been minimized.

Contributor

kalabiyau commented Feb 21, 2012

Just checked on Linux

2.6.38-13-generic #54-Ubuntu SMP Tue Jan 3 13:38:12 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux
Natty

in my case
On Encrypted FS(Cryptfs):

test will fail on filename size >= 144
And will pass on FILENAME_MAX_SIZE = 143

Ubuntu community says that is known filesize bug (on cryptfs) and filesize on current dir underlying fs cannot be determined by syscalls from ruby.
This is related to @tenderlove conversation with Motohiro KOSAKI & Yui NARUSE
on https://bugs.ruby-lang.org/issues/5974

On Ext4 unencrypted:

test will pass on 230 as expected

On FreeBSD UFS unencrypted tests passed on FILENAME_MAX_SIZE = 230

What about cryptfs on Linux - current FILENAME_MAX_SIZE value will make cache files with long names(usually escaped utf strings) unusable before it hooked and truncated by rails.cache api calls

  • Maybe it will be useful to make a hash of limits + fs byte overheads for each FS. But IMO this will be very ugly :)
  • Or get a lowest limit on filename size - 143 for ecryptfs and use it for FILENAME_MAX_SIZE - this is ugly too IMO :(

In my particular case - i only can use a constant override to decrease FILENAME_MAX_SIZE to use a cache on dev and prod machines with cryptfs :( Sad

tenderlove added a commit that referenced this pull request Mar 31, 2012

Merge pull request #4911 from Floppy/master
Reduce FILENAME_MAX_SIZE in ActiveSupport::Cache::FileStore

@tenderlove tenderlove merged commit ad95a61 into rails:master Mar 31, 2012

@chuckbjones

This comment has been minimized.

Contributor

chuckbjones commented Mar 29, 2013

Just ran into this issue on 3.2.11 and ended up overriding the constant to 223. I probably won't submit a pull request to change it in master because 228 should be good enough for most cases, and because we should find a more robust solution as @tenderlove suggests. Maybe catching the ENAMETOOLONG and retrying with a shorter max length, but that may break existing keys.

That said, I'll at least document why I went with 223 instead of 228:
Tempfile adds "#{t}-#{$$}-#{rand(0x100000000).to_s(36)}-#{n}.lock" to the end of the filename.
t is the date; always 8 chars.
$$ is the pid; on linux this defaults to a max of 32768, but can be configured to a max of 4.2million or 7 chars.
rand.to_s base 36 encode of a 32bit number; max of 7 chars
n is the retry count; should never happen, but lets say it could get up to 2 digits before the universe explodes.
That adds up to 32 characters, and 255 - 32 = 223.

So, 228 should work fine in most real world cases where the max pid is 5 digits long and there are no retries and the max filename length is 255.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment