fix(file_sharing): prevent infinite loop on circular symlinks when duplicates are allowed#281
fix(file_sharing): prevent infinite loop on circular symlinks when duplicates are allowed#281jolavillette wants to merge 2 commits intoRetroShare:masterfrom
Conversation
csoler
left a comment
There was a problem hiding this comment.
It seems to me that the AI has tried to re-do what we already do with existing_paths in a new structure (a std::vector, which is bad for find() btw), whereas the problem of circular paths is already handled. What may happen is that if there is a circular symlink, RsDirUtil::removeSymLinks will return an empty string, which is not accounter for. That's probably where the infinite loop comes from.
| RS_DBG4("recursing into \"", stored_dir_it.name()); | ||
|
|
||
| existing_dirs.insert(RsDirUtil::removeSymLinks(stored_dir_it.name())); | ||
| std::string canonical = RsDirUtil::removeSymLinks(stored_dir_it.name()); |
There was a problem hiding this comment.
when there is a loop in the sym links, canonicalize_file_name() will return nullptr and RsDirUtil::removeSymLinks() will return a null std::string (empty()=true). So inserting this empty string in existing_dirs has probably unwanted consequences.
|
|
||
| if( existing_directories.end() != | ||
| existing_directories.find(real_path) ) | ||
| if (std::find(current_branch_real_paths.begin(), current_branch_real_paths.end(), real_path) != current_branch_real_paths.end()) |
There was a problem hiding this comment.
Bad idea to do a find on a std::vector. std::set would be wiser.
…plicates are allowed
…te cycle detection from duplicate filtering
be8a73b to
d73b18b
Compare
|
AI says Thanks for the review! Here's what this v2 addresses: 1. [removeSymLinks] NULL crash — fixed ✅ You were right: 2. Agreed, 3. Why The
The logic is now cleanly separated into 3 levels:
|
fix(file_sharing): prevent infinite loop on circular symlinks when duplicates are allowed