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

fwalk breaks on dangling symlinks #58978

Closed
hynek opened this issue May 10, 2012 · 7 comments
Closed

fwalk breaks on dangling symlinks #58978

hynek opened this issue May 10, 2012 · 7 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@hynek
Copy link
Member

hynek commented May 10, 2012

BPO 14773
Nosy @hynek
Files
  • make-fwalk-handle-dangling-symlinks.diff
  • fwalk-ignore-missing-files.diff
  • fwalk.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/hynek'
    closed_at = <Date 2012-05-16.09:36:17.858>
    created_at = <Date 2012-05-10.19:28:39.968>
    labels = ['type-bug', 'library']
    title = 'fwalk breaks on dangling symlinks'
    updated_at = <Date 2012-05-16.09:39:11.067>
    user = 'https://github.com/hynek'

    bugs.python.org fields:

    activity = <Date 2012-05-16.09:39:11.067>
    actor = 'hynek'
    assignee = 'hynek'
    closed = True
    closed_date = <Date 2012-05-16.09:36:17.858>
    closer = 'hynek'
    components = ['Library (Lib)']
    creation = <Date 2012-05-10.19:28:39.968>
    creator = 'hynek'
    dependencies = []
    files = ['25523', '25556', '25596']
    hgrepos = []
    issue_num = 14773
    keywords = ['patch']
    message_count = 7.0
    messages = ['160364', '160474', '160488', '160499', '160515', '160725', '160730']
    nosy_count = 3.0
    nosy_names = ['neologix', 'python-dev', 'hynek']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue14773'
    versions = ['Python 3.3']

    @hynek
    Copy link
    Member Author

    hynek commented May 10, 2012

    I'm implementing a safe rmtree using fwalk. Everything works perfectly except for one thing: if the directory contains dangling symlinks, fwalk goes belly-up:

    $ ls -l test/
    total 0
    lrwxrwxrwx 1 vagrant vagrant 4 May 10 16:36 doesntwork -> this
    $ ./python
    Python 3.3.0a3+ (default:b32baa5b7626+, May 10 2012, 14:56:20) 
    [GCC 4.6.3] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import os
    [71253 refs]
    >>> list(os.fwalk('test'))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/vagrant/p/Lib/os.py", line 342, in fwalk
        for x in _fwalk(topfd, top, topdown, onerror, followlinks):
      File "/home/vagrant/p/Lib/os.py", line 361, in _walk
        if st.S_ISDIR(fstatat(topfd, name).st_mode):
    FileNotFoundError: [Errno 2] No such file or directory

    Unfortunately this makes it impossible to implement rmtree. The reason is the following code:

    for name in names:
        # Here, we don't use AT_SYMLINK_NOFOLLOW to be consistent with
        # walk() which reports symlinks to directories as directories. We do
        # however check for symlinks before recursing into a subdirectory.
        if st.S_ISDIR(fstatat(topfd, name).st_mode):
            dirs.append(name)
        else:
            nondirs.append(name)

    The "unsafe" walk tree uses os.path.isdir() instead of os.fstatat() and handles this case gracefully.

    A simple try-except protection with a symlink check fixes it and the tests pass. This is a blocker for bpo-4489. I have expanded the test of the WalkerTests suite.

    Tested on Linux (= works) and OS X (= skipped).

    @hynek hynek self-assigned this May 10, 2012
    @hynek hynek added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels May 10, 2012
    @neologix
    Copy link
    Mannequin

    neologix mannequin commented May 12, 2012

    I'm not sure we really need to check for a dangling symlink in case of FileNotFoundError: whether it's a dangling symlink, or the file disappeared in-between-, skipping it is OK.

    @hynek
    Copy link
    Member Author

    hynek commented May 12, 2012

    But if it is a dangling symlink, you want to add it to nondirs while missing files could be skipped, no? You can't skip dangling symlinks if you want to implement rmtree. The normal walk() doesn't too.

    @hynek
    Copy link
    Member Author

    hynek commented May 12, 2012

    So I've changed the patch to ignore everything missing except for dangling links (which throw unfortunately the same exception).

    Just to stress it once more: a fwalk that _ignores_ dangling symlinks is worthless for rmtree. And wasn't rmtree the initial reason to implement fwalk in the first place? ;)

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented May 13, 2012

    Just to stress it once more: a fwalk that _ignores_ dangling symlinks is worthless for rmtree. And wasn't rmtree the initial reason to implement fwalk in the first place? ;)

    Indeed, my bad :-)

    So I've changed the patch to ignore everything missing except for dangling links (which throw unfortunately the same exception).

    Looks good to me.

    @hynek
    Copy link
    Member Author

    hynek commented May 15, 2012

    I just realized it doesn't really make sense because if a file disappears for real, we'll get another FileNotFoundException when checking whether it's a symlink and the continue is never reached.

    So behold v3. :)

    This time, I have tested it by injecting a

    if name == 'tmp4':
        import os
        os.unlinkat(topfd, name)

    right before the S_ISDIR in fwalk.

    Some tests failed because said tmp4 was obviously missing – the old code threw FileNotFoundExceptions. After restoration the whole test suite passes in regression mode.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 15, 2012

    New changeset cbe7560d4443 by Hynek Schlawack in branch 'default':
    bpo-14773: Fix os.fwalk() failing on dangling symlinks
    http://hg.python.org/cpython/rev/cbe7560d4443

    @hynek hynek closed this as completed May 16, 2012
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant