Merge pull request #2920 from idodeclare/bugfix/symlink_laxness

Bugfix/symlink laxness

Author: tarzanek

opengrok-indexer/pom.xml

@@ -19,7 +19,7 @@ information: Portions Copyright [yyyy] [name of copyright owner]
 CDDL HEADER END
 
 Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved.
-Portions Copyright (c) 2017-2018, Chris Fraire <cfraire@me.com>.
+Portions Copyright (c) 2017-2019, Chris Fraire <cfraire@me.com>.
 
 -->
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
@@ -51,6 +51,11 @@ Portions Copyright (c) 2017-2018, Chris Fraire <cfraire@me.com>.
             <artifactId>commons-lang3</artifactId>
             <version>${apache-commons-lang3.version}</version>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-compress</artifactId>
+            <version>1.19</version>
+        </dependency>
         <dependency>
             <groupId>org.apache.lucene</groupId>
             <artifactId>lucene-core</artifactId>

opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/AnalyzerGuru.java

@@ -112,7 +112,6 @@
 import org.opengrok.indexer.history.HistoryReader;
 import org.opengrok.indexer.logger.LoggerFactory;
 import org.opengrok.indexer.search.QueryBuilder;
-import org.opengrok.indexer.util.ForbiddenSymlinkException;
 import org.opengrok.indexer.util.IOUtils;
 import org.opengrok.indexer.web.Util;
 
@@ -527,12 +526,10 @@ public static void returnAnalyzers() {
      * @param xrefOut Where to write the xref (possibly {@code null})
      * @throws IOException If an exception occurs while collecting the data
      * @throws InterruptedException if a timeout occurs
-     * @throws ForbiddenSymlinkException if symbolic-link checking encounters
-     * an ineligible link
      */
     public void populateDocument(Document doc, File file, String path,
         AbstractAnalyzer fa, Writer xrefOut) throws IOException,
-            InterruptedException, ForbiddenSymlinkException {
+            InterruptedException {
 
         String date = DateTools.timeToString(file.lastModified(),
                 DateTools.Resolution.MILLISECOND);

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java

@@ -19,7 +19,7 @@
 
 /*
  * Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved.
- * Portions Copyright (c) 2017-2018, Chris Fraire <cfraire@me.com>.
+ * Portions Copyright (c) 2017-2019, Chris Fraire <cfraire@me.com>.
  */
 package org.opengrok.indexer.configuration;
 
@@ -201,6 +201,7 @@ public final class Configuration {
     private String CTagsExtraOptionsFile;
     private int scanningDepth;
     private Set<String> allowedSymlinks;
+    private Set<String> canonicalRoots;
     private boolean obfuscatingEMailAddresses;
     private boolean chattyStatusPage;
     private final Map<String, String> cmds;  // repository type -> command
@@ -441,6 +442,7 @@ public Configuration() {
         //setBugPage("http://bugs.myserver.org/bugdatabase/view_bug.do?bug_id=");
         setBugPattern("\\b([12456789][0-9]{6})\\b");
         setCachePages(5);
+        setCanonicalRoots(new HashSet<>());
         setCommandTimeout(600); // 10 minutes
         setInteractiveCommandTimeout(30);
         setCompressXref(true);
@@ -1173,6 +1175,14 @@ public void setAllowedSymlinks(Set<String> allowedSymlinks) {
         this.allowedSymlinks = allowedSymlinks;
     }
 
+    public Set<String> getCanonicalRoots() {
+        return canonicalRoots;
+    }
+
+    public void setCanonicalRoots(Set<String> canonicalRoots) {
+        this.canonicalRoots = canonicalRoots;
+    }
+
     public boolean isObfuscatingEMailAddresses() {
         return obfuscatingEMailAddresses;
     }

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java

@@ -19,7 +19,7 @@
 
  /*
   * Copyright (c) 2006, 2019, Oracle and/or its affiliates. All rights reserved.
-  * Portions Copyright (c) 2017-2018, Chris Fraire <cfraire@me.com>.
+  * Portions Copyright (c) 2017-2019, Chris Fraire <cfraire@me.com>.
   */
 package org.opengrok.indexer.configuration;
 
@@ -419,12 +419,32 @@ public void setSourceRoot(String sourceRoot) {
      * @return Path relative to source root
      * @throws IOException If an IO error occurs
      * @throws FileNotFoundException if the file is not relative to source root
+     * or if {@code sourceRoot} is not defined
      * @throws ForbiddenSymlinkException if symbolic-link checking encounters
      * an ineligible link
      */
     public String getPathRelativeToSourceRoot(File file)
             throws IOException, ForbiddenSymlinkException {
-        return PathUtils.getPathRelativeToSourceRoot(file, 0);
+        String sourceRoot = getSourceRootPath();
+        if (sourceRoot == null) {
+            throw new FileNotFoundException("sourceRoot is not defined");
+        }
+
+        String maybeRelPath = PathUtils.getRelativeToCanonical(file.getPath(),
+                sourceRoot, getAllowedSymlinks(), getCanonicalRoots());
+        File maybeRelFile = new File(maybeRelPath);
+        if (!maybeRelFile.isAbsolute()) {
+            /*
+             * N.b. OpenGrok has a weird convention that source-root "relative"
+             * paths must start with a '/' as they are elsewhere directly
+             * appended to getSourceRootPath() and also stored as such.
+             */
+            maybeRelPath = File.separator + maybeRelPath;
+            return maybeRelPath;
+        }
+
+        throw new FileNotFoundException("Failed to resolve [" + file.getPath()
+                + "] relative to source root [" + sourceRoot + "]");
     }
 
     /**
@@ -1189,6 +1209,15 @@ public void setAllowedSymlinks(Set<String> allowedSymlinks) {
         setConfigurationValue("allowedSymlinks", allowedSymlinks);
     }
 
+    @SuppressWarnings("unchecked")
+    public Set<String> getCanonicalRoots() {
+        return (Set<String>) getConfigurationValue("canonicalRoots");
+    }
+
+    public void setCanonicalRoots(Set<String> canonicalRoots) {
+        setConfigurationValue("canonicalRoots", canonicalRoots);
+    }
+
     /**
      * Return whether e-mail addresses should be obfuscated in the xref.
      * @return if we obfuscate emails
@@ -1456,8 +1485,7 @@ private void generateProjectRepositoriesMap() throws IOException {
             Project proj;
             String repoPath;
             try {
-                repoPath = PathUtils.getPathRelativeToSourceRoot(
-                        new File(r.getDirectoryName()), 0);
+                repoPath = getPathRelativeToSourceRoot(new File(r.getDirectoryName()));
             } catch (ForbiddenSymlinkException e) {
                 LOGGER.log(Level.FINER, e.getMessage());
                 continue;

opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java

@@ -30,6 +30,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Paths;
 import java.text.ParseException;
 import java.util.ArrayList;
@@ -38,7 +39,6 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.TreeSet;
-import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
@@ -166,10 +166,11 @@ Executor getRenamedFilesExecutor(final File file, String sinceRevision) throws I
             cmd.add(sinceRevision + "..");
         }
 
-        if (file.getCanonicalPath().length() > getDirectoryName().length() + 1) {
+        String canonicalPath = file.getCanonicalPath();
+        if (canonicalPath.length() > getCanonicalDirectoryName().length() + 1) {
             // this is a file in the repository
             cmd.add("--");
-            cmd.add(file.getCanonicalPath().substring(getDirectoryName().length() + 1));
+            cmd.add(getPathRelativeToCanonicalRepositoryRoot(canonicalPath));
         }
 
         return new Executor(cmd, new File(getDirectoryName()), sinceRevision != null);
@@ -195,9 +196,8 @@ private HistoryRevResult getHistoryRev(
              * Be careful, git uses only forward slashes in its command and output (not in file path).
              * Using backslashes together with git show will get empty output and 0 status code.
              */
-            String filename = Paths.get(getDirectoryName()).relativize(Paths.get(fullpath))
-                                   .toString()
-                                   .replace(File.separatorChar, '/');
+            String filename = Paths.get(getCanonicalDirectoryName()).relativize(
+                    Paths.get(fullpath)).toString().replace(File.separatorChar, '/');
             ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK);
             String[] argv = {
                 RepoCommand,
@@ -234,13 +234,9 @@ boolean getHistoryGet(
         String fullpath;
         try {
             fullpath = new File(parent, basename).getCanonicalPath();
-        } catch (IOException exp) {
-            LOGGER.log(Level.SEVERE, exp, new Supplier<String>() {
-                @Override
-                public String get() {
-                    return String.format("Failed to get canonical path: %s/%s", parent, basename);
-                }
-            });
+        } catch (IOException e) {
+            LOGGER.log(Level.WARNING, e, () -> String.format(
+                    "Failed to get canonical path: %s/%s", parent, basename));
             return false;
         }
 
@@ -255,17 +251,21 @@ public String get() {
             try {
                 origpath = findOriginalName(fullpath, rev);
             } catch (IOException exp) {
-                LOGGER.log(Level.SEVERE, exp, new Supplier<String>() {
-                    @Override
-                    public String get() {
-                        return String.format("Failed to get original revision: %s/%s (revision %s)", parent, basename, rev);
-                    }
-                });
+                LOGGER.log(Level.SEVERE, exp, () -> String.format(
+                        "Failed to get original revision: %s/%s (revision %s)",
+                        parent, basename, rev));
                 return false;
             }
 
             if (origpath != null) {
-                final String fullRenamedPath = Paths.get(getDirectoryName(), origpath).toString();
+                String fullRenamedPath;
+                try {
+                    fullRenamedPath = Paths.get(getCanonicalDirectoryName(), origpath).toString();
+                } catch (IOException e) {
+                    LOGGER.log(Level.WARNING, e, () -> String.format(
+                            "Failed to get canonical path: .../%s", origpath));
+                    return false;
+                }
                 if (!fullRenamedPath.equals(fullpath)) {
                     result = getHistoryRev(sink, fullRenamedPath, rev);
                 }
@@ -281,19 +281,22 @@ public String get() {
      *
      * @param input a stream with the output from a log or blame command
      * @return a reader that reads the input
-     * @throws IOException if the reader cannot be created
      */
-    static Reader newLogReader(InputStream input) throws IOException {
+    static Reader newLogReader(InputStream input) {
         // Bug #17731: Git always encodes the log output using UTF-8 (unless
         // overridden by i18n.logoutputencoding, but let's assume that hasn't
         // been done for now). Create a reader that uses UTF-8 instead of the
         // platform's default encoding.
-        return new InputStreamReader(input, "UTF-8");
+        return new InputStreamReader(input, StandardCharsets.UTF_8);
     }
 
-    private String getPathRelativeToRepositoryRoot(String fullpath) {
-        String repoPath = getDirectoryName() + File.separator;
-        return fullpath.replace(repoPath, "");
+    private String getPathRelativeToCanonicalRepositoryRoot(String fullpath)
+            throws IOException {
+        String repoPath = getCanonicalDirectoryName() + File.separator;
+        if (fullpath.startsWith(repoPath)) {
+            return fullpath.substring(repoPath.length());
+        }
+        return fullpath;
     }
 
     /**
@@ -304,11 +307,11 @@ private String getPathRelativeToRepositoryRoot(String fullpath) {
      * @param changeset changeset
      * @return original filename relative to the repository root
      * @throws java.io.IOException if I/O exception occurred
-     * @see #getPathRelativeToRepositoryRoot(String)
+     * @see #getPathRelativeToCanonicalRepositoryRoot(String)
      */
-    protected String findOriginalName(String fullpath, String changeset)
+    String findOriginalName(String fullpath, String changeset)
             throws IOException {
-        if (fullpath == null || fullpath.isEmpty() || fullpath.length() < getDirectoryName().length()) {
+        if (fullpath == null || fullpath.isEmpty()) {
             throw new IOException(String.format("Invalid file path string: %s", fullpath));
         }
 
@@ -317,7 +320,7 @@ protected String findOriginalName(String fullpath, String changeset)
                     fullpath, changeset));
         }
 
-        String fileInRepo = getPathRelativeToRepositoryRoot(fullpath);
+        String fileInRepo = getPathRelativeToCanonicalRepositoryRoot(fullpath);
 
         /*
          * Get the list of file renames for given file to the specified
@@ -445,7 +448,7 @@ public Annotation annotate(File file, String revision) throws IOException {
             }
         }
         cmd.add("--");
-        cmd.add(getPathRelativeToRepositoryRoot(file.getCanonicalPath()));
+        cmd.add(getPathRelativeToCanonicalRepositoryRoot(file.getCanonicalPath()));
 
         Executor exec = new Executor(cmd, new File(getDirectoryName()),
                 RuntimeEnvironment.getInstance().getInteractiveCommandTimeout());

opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java

@@ -68,6 +68,8 @@ public final class HistoryGuru {
      */
     private static final HistoryGuru INSTANCE = new HistoryGuru();
 
+    private final RuntimeEnvironment env;
+
     /**
      * The history cache to use.
      */
@@ -91,10 +93,10 @@ public final class HistoryGuru {
      * control system.
      */
     private HistoryGuru() {
-        HistoryCache cache = null;
-        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+        env = RuntimeEnvironment.getInstance();
         scanningDepth = env.getScanningDepth();
 
+        HistoryCache cache = null;
         if (env.useHistoryCache()) {
             cache = new FileHistoryCache();
 
@@ -244,7 +246,7 @@ public History getHistory(File file, boolean withFiles, boolean ui)
         final File dir = file.isDirectory() ? file : file.getParentFile();
         final Repository repo = getRepository(dir);
 
-        RemoteSCM rscm = RuntimeEnvironment.getInstance().getRemoteScmSupported();
+        RemoteSCM rscm = env.getRemoteScmSupported();
         boolean doRemote = (ui && (rscm == RemoteSCM.UIONLY))
                 || (rscm == RemoteSCM.ON)
                 || (ui || ((rscm == RemoteSCM.DIRBASED) && (repo != null) && repo.hasHistoryForDirectories()));
@@ -318,9 +320,9 @@ public boolean hasHistory(File file) {
 
         // This should return true for Annotate view.
         return repo.isWorking() && repo.fileHasHistory(file)
-                && ((RuntimeEnvironment.getInstance().getRemoteScmSupported() == RemoteSCM.ON)
-                || (RuntimeEnvironment.getInstance().getRemoteScmSupported() == RemoteSCM.UIONLY)
-                || (RuntimeEnvironment.getInstance().getRemoteScmSupported() == RemoteSCM.DIRBASED)
+                && ((env.getRemoteScmSupported() == RemoteSCM.ON)
+                || (env.getRemoteScmSupported() == RemoteSCM.UIONLY)
+                || (env.getRemoteScmSupported() == RemoteSCM.DIRBASED)
                 || !repo.isRemote());
     }
 
@@ -417,9 +419,11 @@ private Collection<RepositoryInfo> addRepositories(File[] files,
                 } catch (IllegalAccessException iae) {
                     LOGGER.log(Level.WARNING, "Could not create repository for '"
                             + file + "', missing access rights.", iae);
+                    continue;
                 } catch (ForbiddenSymlinkException e) {
-                    LOGGER.log(Level.WARNING, "Could not create repository for '"
-                            + file + "', path traversal issues.", e);
+                    LOGGER.log(Level.WARNING, "Could not create repository for ''{0}'': {1}",
+                            new Object[] {file, e.getMessage()});
+                    continue;
                 }
                 if (repository == null) {
                     // Not a repository, search its sub-dirs.
@@ -541,8 +545,7 @@ private void createCache(Repository repository, String sinceRevision) {
 
     private void createCacheReal(Collection<Repository> repositories) {
         Statistics elapsed = new Statistics();
-        ExecutorService executor = RuntimeEnvironment.getInstance().
-                getIndexerParallelizer().getHistoryExecutor();
+        ExecutorService executor = env.getIndexerParallelizer().getHistoryExecutor();
         // Since we know each repository object from the repositories
         // collection is unique, we can abuse HashMap to create a list of
         // repository,revision tuples with repository as key (as the revision
@@ -704,7 +707,7 @@ public void createCache() {
      */
     private List<Repository> getReposFromString(Collection<String> repositories) {
         ArrayList<Repository> repos = new ArrayList<>();
-        File srcRoot = RuntimeEnvironment.getInstance().getSourceRootFile();
+        File srcRoot = env.getSourceRootFile();
 
         for (String file : repositories) {
             File f = new File(srcRoot, file);

opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryFactory.java

@@ -123,14 +123,16 @@ public static Repository getRepository(File file, boolean interactive)
     public static Repository getRepository(File file, boolean interactive, boolean isNested)
             throws InstantiationException, IllegalAccessException, NoSuchMethodException, InvocationTargetException,
             IOException, ForbiddenSymlinkException {
+
         RuntimeEnvironment env = RuntimeEnvironment.getInstance();
-        Repository repo = null;
+        String relFile = env.getPathRelativeToSourceRoot(file);
 
+        Repository repo = null;
         for (Repository rep : repositories) {
             if ((!isNested || rep.isNestable()) && rep.isRepositoryFor(file, interactive)) {
                 repo = rep.getClass().getDeclaredConstructor().newInstance();
 
-                if (env.isProjectsEnabled() && env.getPathRelativeToSourceRoot(file).equals(File.separator)) {
+                if (env.isProjectsEnabled() && relFile.equals(File.separator)) {
                     LOGGER.log(Level.WARNING, "{0} was detected as {1} repository however with directory " +
                             "matching source root. This is invalid because projects are enabled. Ignoring this " +
                             "repository.",