bnnm / wwiser

wwiser - Wwise .bnk explorer and audio simulator.
182 stars 9 forks source link

Disallow filenames >255 (or 240) chars regardless of platform #5

Closed apocalyptech closed 3 years ago

apocalyptech commented 3 years ago

So I ran into some filenames which were too long on Linux, and a perusal of https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits suggests that support for filenames longer than 255 characters is practically unheard of (though there are a couple exceptions). I see that the code is already enforcing a maximum 240-char full path on Windows (which might not be necessary, given that link above?), but I've added in a 240-char limit on the filename itself, as well. I was using this patch to do so:

diff --git a/wwiser/wtxtp.py b/wwiser/wtxtp.py
index 4b52731..aa6cd4f 100644
--- a/wwiser/wtxtp.py
+++ b/wwiser/wtxtp.py
@@ -6,6 +6,9 @@ from . import wgamesync, wtxtp_tree, wtxtp_info, wversion
 #long paths can be enabled on Windows but detection+support is messy...
 WINDOWS_MAX_PATH = 240

+#use a bit less than 255 for "base" filenames, too.
+MAX_FILENAME_LENGTH = 240
+
 # Builds a TXTP tree from original CAkSound/etc nodes, recreated as a playlist to simplify generation.
 #
 # For example a path like this:
@@ -224,6 +227,14 @@ class Txtp(object):
             outdir = os.path.join(self._basepath, outdir)
             os.makedirs(outdir, exist_ok=True)

+        # https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
+        # ^ This suggests that enforcing a maximum 255-char filename is a pretty
+        # safe assumption.  Very few FSes support more than that.
+        if len(name) > MAX_FILENAME_LENGTH:
+            if not longname:
+                longname = name
+            name = "%s~%04i%s" % (name[0:MAX_FILENAME_LENGTH], self.txtpcache.created, '.txtp')
+
         outname = outdir + name

         info = self._get_info(name, longname)

... the filenames there would actually be likely to approach 250 chars or so, since I just cut it off at 240 and then add the .created index plus .txtp. If you wanted this as a PR instead, I could clean it up and send it through that way. I wasn't sure if you'd like this implementation at all, though, or if you'd approve of my hijacking of longname in there. I also wonder if the Windows-specific check should be taken out, unless there really is a tighter pathname limit than I'm aware of.

apocalyptech commented 3 years ago

Ah, well now that I've had more time to think, I do remember that folks were having issues with pathname lengths when extracting game data, if they'd started too "deep." So yeah, that Windows path checking must be a thing. Anyway, let me know if you'd like this cleaned up and submitted anyway. :)

bnnm commented 3 years ago

Didn't realize Linux had a 255 filename limit so it'd be good to add (haven't tested on Linux actually).

Windows does support long paths, but some programs have trouble handling that, python being one of them (will raise exceptions with paths longer than 255, even if filenames are less).

Feel free to submit (or I can add it if you prefer), probably needs to include self.txtpcache.trims += 1 + logging, and maybe set a flag so Windows path trimming doesn't add +1 again.

apocalyptech commented 3 years ago

Okay, sure thing -- I'll take a stab at doing this properly, then. :) Will probably be a day or two!

bnnm commented 3 years ago

went ahead and added that code since it was just a few lines

apocalyptech commented 3 years ago

Oh shoot, sorry; I'd said I was going to work on it and then ended up totally forgetting about it. :) Thx for merging it in, cheers!