From 1f86350c5a97d8c6966fe1146d649eb5cbc60f53 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Fri, 6 May 2016 13:18:38 -0400 Subject: [PATCH] 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 Signed-off-by: Matthias Sohn --- org.eclipse.jgit.test/META-INF/MANIFEST.MF | 1 + .../tst/org/eclipse/jgit/lib/ConfigTest.java | 83 +++++++++++++++++++ .../eclipse/jgit/internal/JGitText.properties | 2 + .../org/eclipse/jgit/internal/JGitText.java | 2 + .../src/org/eclipse/jgit/lib/Config.java | 43 +++++++++- 5 files changed, 130 insertions(+), 1 deletion(-) diff --git a/org.eclipse.jgit.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.test/META-INF/MANIFEST.MF index af357740cd..0dc6b61586 100644 --- 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 6c6100e116..6d07d34d3a 100644 --- 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 a99bd7e911..f7d9374bd7 100644 --- 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 dc640fe314..d07c001b60 100644 --- 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 70c3997d52..b8eba3acc2 100644 --- 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 fromTextRecurse(final String text, int depth) + throws ConfigInvalidException { + if (depth > MAX_DEPTH) { + throw new ConfigInvalidException( + JGitText.get().tooManyIncludeRecursions); + } final List newEntries = new ArrayList(); 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 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() { -- 2.39.5