Skip to content

Separate max_len functionality into max_filepath_len and max_filename_len #52

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Separate max_len functionality into max_filepath_len and max_filename_len #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2fd0804
Distinguish between filepath and filepath component lengths, deprecat…
7x11x13 Jul 15, 2024
27af09d
Fix filepath validation
7x11x13 Jul 15, 2024
a58201c
Fix tests
7x11x13 Jul 15, 2024
e3fdafc
Update tests for max filename and max filepath lengths
7x11x13 Jul 15, 2024
c8cc600
Undeprecate max_len, remove max_filepath_len for filename classes
7x11x13 Jul 28, 2024
871df1e
Remove deprecation warning
7x11x13 Jul 28, 2024
b029675
Make documentation similar to what it was before
7x11x13 Jul 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 58 additions & 25 deletions pathvalidate/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class BaseFile:
_INVALID_WIN_FILENAME_CHARS: ClassVar[str] = (
_INVALID_FILENAME_CHARS + _INVALID_WIN_PATH_CHARS + "\\"
)
_DEFAULT_MAX_FILENAME_LEN = 255

@property
def platform(self) -> Platform:
Expand All @@ -32,14 +33,24 @@ def reserved_keywords(self) -> Tuple[str, ...]:

@property
def max_len(self) -> int:
return self._max_len
return self._max_filepath_len

@property
def max_filename_len(self) -> int:
return self._max_filename_len

@property
def max_filepath_len(self) -> int:
return self._max_filepath_len

def __init__(
self,
max_len: int,
fs_encoding: Optional[str],
max_filename_len: Optional[int] = None,
max_filepath_len: Optional[int] = None,
additional_reserved_names: Optional[Sequence[str]] = None,
platform_max_len: Optional[int] = None,
platform_max_filename_len: Optional[int] = None,
platform_max_filepath_len: Optional[int] = None,
platform: Optional[PlatformType] = None,
) -> None:
if additional_reserved_names is None:
Expand All @@ -48,15 +59,25 @@ def __init__(

self.__platform = normalize_platform(platform)

if platform_max_len is None:
platform_max_len = self._get_default_max_path_len()
# determine max filepath length
if platform_max_filepath_len is None:
platform_max_filepath_len = self._get_default_max_path_len()

if max_len <= 0:
self._max_len = platform_max_len
if max_filepath_len is None or max_filepath_len <= 0:
self._max_filepath_len = platform_max_filepath_len
else:
self._max_len = max_len
self._max_filepath_len = min(max_filepath_len, platform_max_filepath_len)
Copy link
Contributor Author

@7x11x13 7x11x13 Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something I noticed while writing this is that the current implementation effectively ignores any max_len (or max_filepath_len) value which is higher than the platform's max path length (thus putting a hard limit on max_filepath_len of 4096). I think it would be better not to do this, but changing it would be a breaking change.


self._max_len = min(self._max_len, platform_max_len)
# determine max filename length
if platform_max_filename_len is None:
platform_max_filename_len = self._get_default_max_name_len()

if max_filename_len is None or max_filename_len <= 0:
self._max_filename_len = platform_max_filename_len
else:
self._max_filename_len = max_filename_len
# name cannot be longer than max path length
self._max_filename_len = min(self._max_filename_len, self._max_filepath_len)

if fs_encoding:
self._fs_encoding = fs_encoding
Expand Down Expand Up @@ -99,24 +120,29 @@ def _get_default_max_path_len(self) -> int:

return 260 # universal

def _get_default_max_name_len(self) -> int:
return self._DEFAULT_MAX_FILENAME_LEN


class AbstractValidator(BaseFile, metaclass=abc.ABCMeta):
def __init__(
self,
max_len: int,
fs_encoding: Optional[str],
check_reserved: bool,
max_filename_len: Optional[int] = None,
max_filepath_len: Optional[int] = None,
additional_reserved_names: Optional[Sequence[str]] = None,
platform_max_len: Optional[int] = None,
platform_max_filepath_len: Optional[int] = None,
platform: Optional[PlatformType] = None,
) -> None:
self._check_reserved = check_reserved

super().__init__(
max_len,
fs_encoding,
max_filename_len=max_filename_len,
max_filepath_len=max_filepath_len,
additional_reserved_names=additional_reserved_names,
platform_max_len=platform_max_len,
platform_max_filepath_len=platform_max_filepath_len,
platform=platform,
)

Expand Down Expand Up @@ -145,21 +171,23 @@ class AbstractSanitizer(BaseFile, metaclass=abc.ABCMeta):
def __init__(
self,
validator: AbstractValidator,
max_len: int,
fs_encoding: Optional[str],
validate_after_sanitize: bool,
max_filename_len: Optional[int] = None,
max_filepath_len: Optional[int] = None,
null_value_handler: Optional[ValidationErrorHandler] = None,
reserved_name_handler: Optional[ValidationErrorHandler] = None,
additional_reserved_names: Optional[Sequence[str]] = None,
platform_max_len: Optional[int] = None,
platform_max_filepath_len: Optional[int] = None,
platform: Optional[PlatformType] = None,
) -> None:
super().__init__(
max_len=max_len,
fs_encoding=fs_encoding,
additional_reserved_names=additional_reserved_names,
platform_max_len=platform_max_len,
platform_max_filepath_len=platform_max_filepath_len,
platform=platform,
max_filename_len=max_filename_len,
max_filepath_len=max_filepath_len,
)

if null_value_handler is None:
Expand Down Expand Up @@ -187,23 +215,25 @@ def min_len(self) -> int:
def __init__(
self,
min_len: int,
max_len: int,
fs_encoding: Optional[str],
check_reserved: bool,
max_filename_len: Optional[int] = None,
max_filepath_len: Optional[int] = None,
additional_reserved_names: Optional[Sequence[str]] = None,
platform_max_len: Optional[int] = None,
platform_max_filepath_len: Optional[int] = None,
platform: Optional[PlatformType] = None,
) -> None:
if min_len <= 0:
min_len = DEFAULT_MIN_LEN
self._min_len = max(min_len, 1)

super().__init__(
max_len=max_len,
fs_encoding=fs_encoding,
max_filename_len=max_filename_len,
max_filepath_len=max_filepath_len,
check_reserved=check_reserved,
additional_reserved_names=additional_reserved_names,
platform_max_len=platform_max_len,
platform_max_filepath_len=platform_max_filepath_len,
platform=platform,
)

Expand All @@ -227,11 +257,14 @@ def _validate_reserved_keywords(self, name: str) -> None:
)

def _validate_max_len(self) -> None:
if self.max_len < 1:
raise ValueError("max_len must be greater or equal to one")
if self.max_filename_len < 1:
raise ValueError("max_filename_len must be greater or equal to one")

if self.max_filepath_len < 1:
raise ValueError("max_filepath_len must be greater or equal to one")

if self.min_len > self.max_len:
raise ValueError("min_len must be lower than max_len")
if self.min_len > self.max_filename_len:
raise ValueError("min_len must be lower than max_filename_len")

@staticmethod
def __extract_root_name(path: str) -> str:
Expand Down
Loading