Support git config [include] section with absolute path(s)

As per [1], but limited to absolute paths indeed. No support yet for
tilde or $HOME expansion. Support for the --[no-]includes options
([1]) is not part of this commit scope either, but those options'
defaults are in effect as described in [1].

[1] https://git-scm.com/docs/git-config

Included path can be a config file that includes other path-s in turn.
An exception is thrown if too many recursions (circular includes)
happen because of ill-specified config files.

Change-Id: I700bd7b7e1625eb7de0180f220c707d8e7b0930b
Signed-off-by: Marco Miller <marco.miller@ericsson.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

diff --git a/org.eclipse.jgit.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.test/META-INF/MANIFEST.MF
index af357740cd1b21e16e69727395bca4ade25c6a2d..0dc6b6158697d46ea3626739a52105bd94634cff 100644 (file)
--- a/org.eclipse.jgit.test/META-INF/MANIFEST.MF
+++ b/org.eclipse.jgit.test/META-INF/MANIFEST.MF
@@ -51,6 +51,7 @@ Import-Package: com.googlecode.javaewah;version="[0.7.9,0.8.0)",
  org.hamcrest;version="[1.1.0,2.0.0)",
  org.junit;version="[4.4.0,5.0.0)",
  org.junit.experimental.theories;version="[4.4.0,5.0.0)",
+ org.junit.rules;version="[4.11.0,5.0.0)",
  org.junit.runner;version="[4.4.0,5.0.0)",
  org.junit.runners;version="[4.11.0,5.0.0)",
  org.slf4j;version="[1.7.2,2.0.0)"

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
index 6c6100e11699c837d56e88a071d76bff3107e04c..6d07d34d3a9a48aea9a4498f1fa2a75b20e28f47 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
@@ -56,6 +56,9 @@ import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
 import java.text.MessageFormat;
 import java.util.Arrays;
 import java.util.Iterator;
@@ -64,18 +67,29 @@ import java.util.Set;
 
 import org.eclipse.jgit.api.MergeCommand.FastForwardMode;
 import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.junit.MockSystemReader;
 import org.eclipse.jgit.merge.MergeConfig;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.SystemReader;
 import org.junit.After;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
 
 /**
  * Test reading of git config
  */
 public class ConfigTest {
 
+       @Rule
+       public ExpectedException expectedEx = ExpectedException.none();
+
+       @Rule
+       public TemporaryFolder tmp = new TemporaryFolder();
+
        @After
        public void tearDown() {
                SystemReader.setInstance(null);
@@ -745,6 +759,75 @@ public class ConfigTest {
                assertTrue(c.getBoolean("foo", "bar", false));
        }
 
+       @Test
+       public void testIncludeInvalidName() throws ConfigInvalidException {
+               expectedEx.expect(ConfigInvalidException.class);
+               expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile);
+               parse("[include]\nbar\n");
+       }
+
+       @Test
+       public void testIncludeNoValue() throws ConfigInvalidException {
+               expectedEx.expect(ConfigInvalidException.class);
+               expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile);
+               parse("[include]\npath\n");
+       }
+
+       @Test
+       public void testIncludeEmptyValue() throws ConfigInvalidException {
+               expectedEx.expect(ConfigInvalidException.class);
+               expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile);
+               parse("[include]\npath=\n");
+       }
+
+       @Test
+       public void testIncludeValuePathNotFound() throws ConfigInvalidException {
+               String notFound = "/not/found";
+               expectedEx.expect(ConfigInvalidException.class);
+               expectedEx.expectMessage(notFound);
+               parse("[include]\npath=" + notFound + "\n");
+       }
+
+       @Test
+       public void testIncludeTooManyRecursions() throws IOException {
+               File config = tmp.newFile("config");
+               String include = "[include]\npath=" + config.toPath() + "\n";
+               Files.write(config.toPath(), include.getBytes());
+               FileBasedConfig fbConfig = new FileBasedConfig(null, config,
+                               FS.DETECTED);
+               try {
+                       fbConfig.load();
+                       fail();
+               } catch (ConfigInvalidException cie) {
+                       assertEquals(JGitText.get().tooManyIncludeRecursions,
+                                       cie.getCause().getMessage());
+               }
+       }
+
+       @Test
+       public void testInclude() throws IOException, ConfigInvalidException {
+               File config = tmp.newFile("config");
+               File more = tmp.newFile("config.more");
+               File other = tmp.newFile("config.other");
+
+               String fooBar = "[foo]\nbar=true\n";
+               String includeMore = "[include]\npath=" + more.toPath() + "\n";
+               String includeOther = "path=" + other.toPath() + "\n";
+               String fooPlus = fooBar + includeMore + includeOther;
+               Files.write(config.toPath(), fooPlus.getBytes());
+
+               String fooMore = "[foo]\nmore=bar\n";
+               Files.write(more.toPath(), fooMore.getBytes());
+
+               String otherMore = "[other]\nmore=bar\n";
+               Files.write(other.toPath(), otherMore.getBytes());
+
+               Config parsed = parse("[include]\npath=" + config.toPath() + "\n");
+               assertTrue(parsed.getBoolean("foo", "bar", false));
+               assertEquals("bar", parsed.getString("foo", null, "more"));
+               assertEquals("bar", parsed.getString("other", null, "more"));
+       }
+
        private static void assertReadLong(long exp) throws ConfigInvalidException {
                assertReadLong(exp, String.valueOf(exp));
        }

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index a99bd7e9115d207514f1c86e7996e5ee228d7543..f7d9374bd7fa6ca351cafe0faf0b877e54a33859 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -337,6 +337,7 @@ invalidId0=Invalid id
 invalidIdLength=Invalid id length {0}; should be {1}
 invalidIgnoreParamSubmodule=Found invalid ignore param for submodule {0}.
 invalidIgnoreRule=Exception caught while parsing ignore rule ''{0}''.
+invalidIncludedPathInConfigFile=Invalid included path in config file: {0}
 invalidIntegerValue=Invalid integer value: {0}.{1}={2}
 invalidKey=Invalid key: {0}
 invalidLineInConfigFile=Invalid line in config file
@@ -592,6 +593,7 @@ tagNameInvalid=tag name {0} is invalid
 tagOnRepoWithoutHEADCurrentlyNotSupported=Tag on repository without HEAD currently not supported
 theFactoryMustNotBeNull=The factory must not be null
 timerAlreadyTerminated=Timer already terminated
+tooManyIncludeRecursions=Too many recursions; circular includes in config file(s)?
 topologicalSortRequired=Topological sort required.
 transactionAborted=transaction aborted
 transportExceptionBadRef=Empty ref: {0}: {1}

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index dc640fe314a0741146ce7d9ab6f5b4c25138af72..d07c001b6081ee92a2cfd3c81ed6d4e2c99add26 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -396,6 +396,7 @@ public class JGitText extends TranslationBundle {
        /***/ public String invalidIdLength;
        /***/ public String invalidIgnoreParamSubmodule;
        /***/ public String invalidIgnoreRule;
+       /***/ public String invalidIncludedPathInConfigFile;
        /***/ public String invalidIntegerValue;
        /***/ public String invalidKey;
        /***/ public String invalidLineInConfigFile;
@@ -652,6 +653,7 @@ public class JGitText extends TranslationBundle {
        /***/ public String transactionAborted;
        /***/ public String theFactoryMustNotBeNull;
        /***/ public String timerAlreadyTerminated;
+       /***/ public String tooManyIncludeRecursions;
        /***/ public String topologicalSortRequired;
        /***/ public String transportExceptionBadRef;
        /***/ public String transportExceptionEmptyRef;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
index 70c3997d522ad27f6e29756c30682ce327bb0214..b8eba3acc2a8ea0a4944cdeaa7ca8933e6eb52e1 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
@@ -51,6 +51,8 @@
 
 package org.eclipse.jgit.lib;
 
+import java.io.File;
+import java.io.IOException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -64,6 +66,8 @@ import org.eclipse.jgit.events.ConfigChangedListener;
 import org.eclipse.jgit.events.ListenerHandle;
 import org.eclipse.jgit.events.ListenerList;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.util.IO;
+import org.eclipse.jgit.util.RawParseUtils;
 import org.eclipse.jgit.util.StringUtils;
 
 
@@ -75,6 +79,7 @@ public class Config {
        private static final long KiB = 1024;
        private static final long MiB = 1024 * KiB;
        private static final long GiB = 1024 * MiB;
+       private static final int MAX_DEPTH = 999;
 
        /** the change listeners */
        private final ListenerList listeners = new ListenerList();
@@ -1027,6 +1032,15 @@ public class Config {
         *             made to {@code this}.
         */
        public void fromText(final String text) throws ConfigInvalidException {
+               state.set(newState(fromTextRecurse(text, 1)));
+       }
+
+       private List<ConfigLine> fromTextRecurse(final String text, int depth)
+                       throws ConfigInvalidException {
+               if (depth > MAX_DEPTH) {
+                       throw new ConfigInvalidException(
+                                       JGitText.get().tooManyIncludeRecursions);
+               }
                final List<ConfigLine> newEntries = new ArrayList<ConfigLine>();
                final StringReader in = new StringReader(text);
                ConfigLine last = null;
@@ -1085,11 +1099,38 @@ public class Config {
                                } else
                                        e.value = readValue(in, false, -1);
 
+                               if (e.section.equals("include")) { //$NON-NLS-1$
+                                       addIncludedConfig(newEntries, e, depth);
+                               }
                        } else
                                throw new ConfigInvalidException(JGitText.get().invalidLineInConfigFile);
                }
 
-               state.set(newState(newEntries));
+               return newEntries;
+       }
+
+       private void addIncludedConfig(final List<ConfigLine> newEntries,
+                       ConfigLine line, int depth) throws ConfigInvalidException {
+               if (!line.name.equals("path") || //$NON-NLS-1$
+                               line.value == null || line.value.equals(MAGIC_EMPTY_VALUE)) {
+                       throw new ConfigInvalidException(
+                                       JGitText.get().invalidLineInConfigFile);
+               }
+               File path = new File(line.value);
+               try {
+                       byte[] bytes = IO.readFully(path);
+                       String decoded;
+                       if (isUtf8(bytes)) {
+                               decoded = RawParseUtils.decode(RawParseUtils.UTF8_CHARSET,
+                                               bytes, 3, bytes.length);
+                       } else {
+                               decoded = RawParseUtils.decode(bytes);
+                       }
+                       newEntries.addAll(fromTextRecurse(decoded, depth + 1));
+               } catch (IOException ioe) {
+                       throw new ConfigInvalidException(MessageFormat.format(
+                                       JGitText.get().invalidIncludedPathInConfigFile, path));
+               }
        }
 
        private ConfigSnapshot newState() {