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.

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

Jump to bottom

SourcePath::resolveSymlinks(): Fix handling of symlinks that start with '..' #8452

Merged
merged 1 commit into from
Jun 6, 2023
Merged

SourcePath::resolveSymlinks(): Fix handling of symlinks that start with '..' #8452

merged 1 commit into from
Jun 6, 2023

Conversation

edolstra
Copy link
Member

@edolstra edolstra commented Jun 5, 2023

Motivation

Fixes #8437.

Context

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@github-actions github-actions bot added fetching Networking with the outside (non-Nix) world, input locking with-tests Issues related to testing. PRs with tests have some priority labels Jun 5, 2023
@roberth roberth added the bug label Jun 5, 2023
roberth
roberth reviewed Jun 5, 2023
View reviewed changes
src/libutil/canon-path.hh Outdated Show resolved Hide resolved
@fricklerhandwerk
Copy link
Contributor

Suggestion for commit message: Previously, when resolving a symlink, .. components would not be taken into account.

@edolstra edolstra force-pushed the fix-relative-symlinks branch from 6731850 to 2151be5 Compare June 5, 2023 13:44
@github-actions github-actions bot removed the with-tests Issues related to testing. PRs with tests have some priority label Jun 5, 2023
@infinisil
Copy link
Member

Can you also add a test case to prevent against this bug in the future?

@edolstra
Fix SourcePath::resolveSymlinks()
f5c6b29 
This fixes handling of symlinks that start with '..', and symlink
targets that contain symlinks themselves.
@edolstra edolstra force-pushed the fix-relative-symlinks branch from 2151be5 to f5c6b29 Compare June 6, 2023 09:24
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jun 6, 2023
@edolstra
Copy link
Member Author

edolstra commented Jun 6, 2023

Added a test.

@edolstra edolstra enabled auto-merge June 6, 2023 09:24
@edolstra edolstra merged commit f5ab40c into NixOS:master Jun 6, 2023
@github-actions github-actions bot mentioned this pull request Jun 6, 2023
Merged
@github-actions
Copy link

github-actions bot commented Jun 6, 2023

Successfully created backport PR for 2.16-maintenance:

@edolstra edolstra deleted the fix-relative-symlinks branch June 6, 2023 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roberth roberth roberth left review comments

Assignees
No one assigned
Labels
bug fetching Networking with the outside (non-Nix) world, input locking with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[2.16 regression] "Infinite symlink recursion" when evaluating nixos-hardware
4 participants
@edolstra @fricklerhandwerk @infinisil @roberth