Skip to content
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

Limit length of filenames #197

Closed
fiete201 opened this issue Sep 20, 2017 · 4 comments
Closed

Limit length of filenames #197

fiete201 opened this issue Sep 20, 2017 · 4 comments
Assignees
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels
Milestone

Comments

@fiete201
Copy link

I recently bought some CDs with Music from the Barock age. Those have sometimes Track names longer than 255 chars resulting in ripping Error MAX_RETRIES since file cant be written because in ext4 Filesystems 255 is the maximum length for filenames.

It would be nice to just cut off the track name or give an additional option to limit the length of the filename.

Thanks and best regards
Fritz

@RecursiveForest
Copy link
Contributor

I think this is a good idea, we should automatically truncate track names to the filesystem length limit. I'll have to check what the most pythonic & portable way to inherit this limit from the OS is.

@JoeLametta
Copy link
Collaborator

JoeLametta commented Oct 8, 2017

I'll have to check what the most pythonic & portable way to inherit this limit from the OS is.

What about something like this? (using PC_NAME_MAX and PC_PATH_MAX as name arguments).
For example we can define a function which takes two arguments (filename, fullpath) and checks if both are within the aforementioned limits.

from os import pathconf


def check_path_limits(filename, fullpath):
    """Return true if both the lengths of the filename and fullpath arguments
    are within the limits provided by the OS, false otherwise.
    """
    flen_limit = pathconf(fullpath, 'PC_NAME_MAX')
    plen_limit = pathconf(fullpath, 'PC_PATH_MAX')
    return len(filename) < flen_limit and len(fullpath) < plen_limit

@JoeLametta JoeLametta added this to the 1.0 milestone Apr 6, 2018
@JoeLametta JoeLametta self-assigned this Apr 6, 2018
@JoeLametta JoeLametta added the Status: in progress Issue/pull request which is currently being worked on label May 9, 2018
@JoeLametta
Copy link
Collaborator

@RecursiveForest What about this? (it only truncates filenames)

from os import pathconf


def truncate_filename(filename, extension, path):
    """Truncate filename to the max. len. allowed by the path's filesystem"""
    fn_lim = pathconf(path, 'PC_NAME_MAX')
    length = fn_lim - len(extension)
    return filename[:length]

It should be portable enough.

Availability: Unix.

@JoeLametta
Copy link
Collaborator

Completely untested patch:

From: JoeLametta <JoeLametta@users.noreply.github.com>
Date: Wed, 20 Jun 2018 08:00:00 +0000
Subject: [PATCH] Truncate filenames longer than filesystem's limit

---
 whipper/common/common.py      |  7 +++++++
 whipper/common/program.py     | 12 +++++++++---
 whipper/program/cdparanoia.py |  4 ++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/whipper/common/common.py b/whipper/common/common.py
index c8aefd4..ff23f50 100644
--- a/whipper/common/common.py
+++ b/whipper/common/common.py
@@ -153,6 +153,13 @@ class MissingFrames(Exception):
     pass
 
 
+def truncate_filename(path, filename, extension):
+    """Truncate filename to the max. len. allowed by the path's filesystem"""
+    fn_lim = os.pathconf(path, 'PC_NAME_MAX')
+    length = fn_lim - len(extension)
+    return filename[:length] + extension
+
+
 def shrinkPath(path):
     """
     Shrink a full path to a shorter version.
diff --git a/whipper/common/program.py b/whipper/common/program.py
index c16a660..f0297ec 100644
--- a/whipper/common/program.py
+++ b/whipper/common/program.py
@@ -571,7 +571,9 @@ class Program:
         return accurip.verify_result(self.result, responses, checksums)
 
     def write_m3u(self, discname):
-        m3uPath = u'%s.m3u' % discname
+        p, f = os.path.split(os.path.normpath(discname))
+        safe_name = common.truncate_filename(p, f, '.m3u')
+        m3uPath = os.path.join(p, safe_name)
         with open(m3uPath, 'w') as f:
             f.write(u'#EXTM3U\n'.encode('utf-8'))
             for track in self.result.tracks:
@@ -593,7 +595,9 @@ class Program:
 
     def writeCue(self, discName):
         assert self.result.table.canCue()
-        cuePath = '%s.cue' % discName
+        p, f = os.path.split(os.path.normpath(discName))
+        safe_name = common.truncate_filename(p, f, '.cue')
+        cuePath = os.path.join(p, safe_name)
         logger.debug('write .cue file to %s', cuePath)
         handle = open(cuePath, 'w')
         # FIXME: do we always want utf-8 ?
@@ -605,7 +609,9 @@ class Program:
         return cuePath
 
     def writeLog(self, discName, logger):
-        logPath = '%s.log' % discName
+        p, f = os.path.split(os.path.normpath(discName))
+        safe_name = common.truncate_filename(p, f, '.log')
+        logPath = os.path.join(p, safe_name)
         handle = open(logPath, 'w')
         log = logger.log(self.result)
         handle.write(log.encode('utf-8'))
diff --git a/whipper/program/cdparanoia.py b/whipper/program/cdparanoia.py
index 3163dc7..39cc8fc 100644
--- a/whipper/program/cdparanoia.py
+++ b/whipper/program/cdparanoia.py
@@ -482,6 +482,10 @@ class ReadVerifyTrackTask(task.MultiSeparateTask):
             if errno.ENAMETOOLONG != e.errno:
                 raise
             path = common.shrinkPath(path)
+            p, f = os.path.split(os.path.normpath(path))
+            f, e = os.path.splitext(f)
+            safe_name = common.truncate_filename(p, f, e)
+            path = os.path.join(p, safe_name)
             tmpoutpath = path + u'.part'
             open(tmpoutpath, 'wb').close()
         self._tmppath = tmpoutpath

JoeLametta added a commit that referenced this issue Oct 18, 2018
If whipper generated filenames are longer thant the maximum value supported by the filesystem, the I/O operations are going to fail.
With this commit filenames which may be too long are truncated to the maximum allowable length.

Fixes #197.
@JoeLametta JoeLametta added completed and removed Status: in progress Issue/pull request which is currently being worked on labels Oct 18, 2018
JoeLametta added a commit that referenced this issue Oct 22, 2018
If whipper generated filenames are longer thant the maximum value supported by the filesystem, the I/O operations are going to fail.
With this commit filenames which may be too long are truncated to the maximum allowable length.

Fixes #197.
JoeLametta added a commit that referenced this issue Oct 22, 2018
* Limit length of filenames

If whipper generated filenames are longer thant the maximum value supported by the filesystem, the I/O operations are going to fail.
With this commit filenames which may be too long are truncated to the maximum allowable length.

Fixes #197.
@JoeLametta JoeLametta added Bug Generic bug: can be used together with more specific labels Accepted Accepted issue on our roadmap and removed enhancement labels Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels
Projects
None yet
Development

No branches or pull requests

3 participants