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

Bugfix/symlinks2 #1989

Merged
merged 7 commits into from
Jan 26, 2018
Merged

Bugfix/symlinks2 #1989

merged 7 commits into from
Jan 26, 2018

Conversation

idodeclare
Copy link
Contributor

(This is just reopening PRs #1911, #1952 rebased on master.)

Hello,

Please consider for integration this patch with fixes for symbolic link handling.

The principle addition is PathUtils.getRelativeToCanonical(...) and a new explicit ForbiddenSymlinkException.

When PR #1846 was in progress, my testing showed it still had problems with history processing and also could not support nested symbolic links. I first cherry-picked most of #1846's contents but then revised its versions of RuntimeEnvironment.getPathRelativeToSourceRoot(...) and HistoryGuru.getRepository(...) to instead use the new PathUtils.

I also kept #1846's new tests and added some more.

This patch also contains a new method, Repository.getRepoRelativePath(...), to replace (pre-existing) problematic logic in the various repository implementations (e.g., Git, Mercurial, SVN, etc.) that was lopping off the front of paths assuming they started with "source root". E.g., on macOS, /var is a symlink to /private/var, and the logic was chopping off the number of characters in "/private/var/..." from "/var/...", resulting in paths mangled in the middle of segments.

(I detected the problem while testing scenarios of symbolic-linked project layouts, but unit tests otherwise did not detect this bug.)

TESTING SCENARIOS:

  1. Basic case with no symlinks under SRC_ROOT — this behaves as before.
  2. Direct symlinks under SRC_ROOT. Here of course Indexer "automatically allow symlinks that are directly in source root". HistoryGuru and history classes, though, are now fixed to properly work in this case.
  3. Add another symlink inside a directory referenced by a symlink under SRC_ROOT. Indexer will not have allowed this second symlink, so the new logic properly detects this as disallowed. This repo will not be indexed and will not show up in the UI.
  4. Build on the previous test by adding an OPENGROK_READ_XML_CONFIGURATION file with the secondary symlink added to allowedSymlinks. Indexer will now allow this repo; it is handled by HistoryGuru et al; and the repo will show up in the UI.

Thank you.

Vladimir Kotal and others added 7 commits January 25, 2018 10:47
- HistoryGuru etc. will operate correctly and no
  longer throw exceptions.
- index.jsp will show symlinked projects.
- Fix macOS regression in Oracle PR oracle#1846.
This quiets the noisy exceptions for repos that
heavily use symlinks -- e.g., Homebrew/brew.
Also, restore accidentally removed block comment.
@tarzanek tarzanek self-assigned this Jan 26, 2018
@tarzanek tarzanek added this to the 1.1 milestone Jan 26, 2018
@tarzanek
Copy link
Contributor

merging, thank you sir!

@tarzanek tarzanek merged commit d7b8200 into oracle:master Jan 26, 2018
@idodeclare idodeclare deleted the bugfix/symlinks2 branch January 26, 2018 14:13
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.

2 participants