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

Fix: Avoid infinite recursion when source and destinations are same while using cp -R #3018

Merged
merged 2 commits into from
Feb 20, 2022

Conversation

Narasimha1997
Copy link
Contributor

@Narasimha1997 Narasimha1997 commented Feb 1, 2022

Proposing a solution for #3016
The infinite recursion can be prevented when running `cp -r <source_dir> <destination_dir> by handling these two cases:

1. Source and Destination refer to same canonicalized path, after resolving all symbolic links etc

# Example: 
cp -r /home/ubuntu/parent   /home/ubuntu/parent 

2. Canonicalized destination has a canonicalized source as prefix:

This can also lead to recursion, imagine there is a directory called child under /home/ubuntu/parent (i.e /home/ubuntu/parent/child, then the following example leads to recursion:

cp -r /home/ubuntu/parent /home/ubuntu/parent/child

The official GNU coreutil's cp handles both of these cases.

The above two cases can be solved by simply checking for case 2, i.e don't allow copy if destination has source in it's prefix.
I added a function:

pub fn path_has_prefix(p1: &Path, p2: &Path) -> io::Result<bool> {
    let pathbuf1 = canonicalize(p1, MissingHandling::Normal, ResolveMode::Logical)?;
    let pathbuf2 = canonicalize(p2, MissingHandling::Normal, ResolveMode::Logical)?;

    Ok(pathbuf1.starts_with(pathbuf2))
}

Which checks that and calling it in copy_directory function.

@sylvestre
Copy link
Contributor

Fun, could you please add a test to make sure we don't regress?

@tertsdiepraam tertsdiepraam linked an issue Feb 1, 2022 that may be closed by this pull request
Narasimha1997 added a commit to Narasimha1997/coreutils that referenced this pull request Feb 1, 2022
@Narasimha1997
Copy link
Contributor Author

@sylvestre added tests as you suggested.

@Narasimha1997
Copy link
Contributor Author

Sorry clicked on Close with comment by mistake haha

…ctories are same or source is a prefix of destination
@Narasimha1997
Copy link
Contributor Author

@tertsdiepraam some merge issues with this branch

@tertsdiepraam
Copy link
Member

Can you try to fix them? I think it's just that a test was added at the end of the file in both branches and that we should keep both.

@sylvestre sylvestre merged commit 7305180 into uutils:main Feb 20, 2022
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 this pull request may close these issues.

cp: Copying directory into itself causes unbounded recursion
4 participants