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

Unsafe string slicing in IgnoreRule.match method. It fails with Path type. #37

Closed
antonkei opened this issue Nov 23, 2022 · 6 comments · Fixed by #38
Closed

Unsafe string slicing in IgnoreRule.match method. It fails with Path type. #37

antonkei opened this issue Nov 23, 2022 · 6 comments · Fixed by #38

Comments

@antonkei
Copy link

Synopsis

After upgrade from 0.1.0 to 0.1.1 I faced an issue with existing test. Looks like there is unsafe operation with abs_path which supposed to be a string only. That's a wrong assuming, because most libraries support the both types: [str, Path].

Code impacted: https://github.com/mherrmann/gitignore_parser/blob/v0.1.1/gitignore_parser.py#L143

Steps to reproduce

import tempfile
from pathlib import Path

from gitignore_parser import parse_gitignore


def reproduce_issue():
    with tempfile.TemporaryDirectory() as git_dir:
        git_dir = Path(git_dir)
        gitignore_file_path = git_dir / ".gitignore"
        gitignore_file_path.write_text("file1\n!file2")
        matches = parse_gitignore(gitignore_file_path)

        assert matches(git_dir / "file1")
        assert not matches(git_dir / "file2")


if __name__ == '__main__':
    reproduce_issue()

Observed behaviour

Traceback (most recent call last):
  File "/Users/kukusan2/Workspace/gitignore_issue/reproduce_issue.py", line 20, in <module>
    reproduce_issue()
  File "/Users/kukusan2/Workspace/gitignore_issue/reproduce_issue.py", line 15, in reproduce_issue
    assert matches(git_dir / "file1")
  File "/Users/kukusan2/Workspace/gitignore_issue/venv2/lib/python3.9/site-packages/gitignore_parser.py", line 36, in <lambda>
    return lambda file_path: handle_negation(file_path, rules)
  File "/Users/kukusan2/Workspace/gitignore_issue/venv2/lib/python3.9/site-packages/gitignore_parser.py", line 11, in handle_negation
    if rule.match(file_path):
  File "/Users/kukusan2/Workspace/gitignore_issue/venv2/lib/python3.9/site-packages/gitignore_parser.py", line 143, in match
    if self.negation and abs_path[-1] == '/':
TypeError: 'PosixPath' object is not subscriptable
@mherrmann
Copy link
Owner

I'll be happy to merge a PR with a fix :-) @szczeles what do you think about this issue?

@szczeles
Copy link
Contributor

Good point, I can fix it quickly. Path objects in python do not preserve trailing slash, so the fix is trivial

@antonkei
Copy link
Author

Thank you folks. I really appreciate that.
BWT, I've checked the PR. Why wouldn't you cast all paths to Path from the user's input? After it you needn't to care about its type in the method body.

@szczeles
Copy link
Contributor

Python's Path doesn't say if it is dir or a regular file, it even trims the slash:

>>> from pathlib import Path
>>> Path("/tmp/")
PosixPath('/tmp')
>>> str(Path("/tmp/"))
'/tmp'
>>> 

Therefore, tricks like Kedro uses (directory negation: #36) do not work with Path.

@antonkei
Copy link
Author

Oh, got it. Thank you for the explanation.

@mherrmann
Copy link
Owner

This is fixed on PyPI now in v0.1.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants