From e12c6b58ee0284cbdf34bf325deb561a52d10340 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 27 Jan 2018 16:06:15 +0100 Subject: [PATCH] Minor improvements in git config file inclusions * Section and key names in git config files are case-insensitive. * If an include directive is invalid, include the line in the exception message. * If inclusion of the included file fails, put the file name into the exception message so that the user knows in which file the problem is. Change-Id: If920943af7ff93f5321b3d315dfec5222091256c Signed-off-by: Thomas Wolf --- .../tst/org/eclipse/jgit/lib/ConfigTest.java | 84 ++++++++++++++++++- .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../src/org/eclipse/jgit/lib/Config.java | 15 ++-- 4 files changed, 94 insertions(+), 7 deletions(-) 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 fb1ee8cad..7862005eb 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 @@ -808,8 +808,14 @@ public class ConfigTest { fbConfig.load(); fail(); } catch (ConfigInvalidException cie) { - assertEquals(JGitText.get().tooManyIncludeRecursions, - cie.getCause().getMessage()); + for (Throwable t = cie; t != null; t = t.getCause()) { + if (t.getMessage() + .equals(JGitText.get().tooManyIncludeRecursions)) { + return; + } + } + fail("Expected to find expected exception message: " + + JGitText.get().tooManyIncludeRecursions); } } @@ -824,6 +830,80 @@ public class ConfigTest { assertFalse(parsed.getBoolean("foo", "bar", false)); } + @Test + public void testIncludeCaseInsensitiveSection() + throws IOException, ConfigInvalidException { + File included = tmp.newFile("included"); + String content = "[foo]\nbar=true\n"; + Files.write(included.toPath(), content.getBytes()); + + File config = tmp.newFile("config"); + content = "[Include]\npath=" + pathToString(included) + "\n"; + Files.write(config.toPath(), content.getBytes()); + + FileBasedConfig fbConfig = new FileBasedConfig(null, config, + FS.DETECTED); + fbConfig.load(); + assertTrue(fbConfig.getBoolean("foo", "bar", false)); + } + + @Test + public void testIncludeCaseInsensitiveKey() + throws IOException, ConfigInvalidException { + File included = tmp.newFile("included"); + String content = "[foo]\nbar=true\n"; + Files.write(included.toPath(), content.getBytes()); + + File config = tmp.newFile("config"); + content = "[include]\nPath=" + pathToString(included) + "\n"; + Files.write(config.toPath(), content.getBytes()); + + FileBasedConfig fbConfig = new FileBasedConfig(null, config, + FS.DETECTED); + fbConfig.load(); + assertTrue(fbConfig.getBoolean("foo", "bar", false)); + } + + @Test + public void testIncludeExceptionContainsLine() { + try { + parse("[include]\npath=\n"); + fail("Expected ConfigInvalidException"); + } catch (ConfigInvalidException e) { + assertTrue( + "Expected to find the problem line in the exception message", + e.getMessage().contains("include.path")); + } + } + + @Test + public void testIncludeExceptionContainsFile() throws IOException { + File included = tmp.newFile("included"); + String includedPath = pathToString(included); + String content = "[include]\npath=\n"; + Files.write(included.toPath(), content.getBytes()); + + File config = tmp.newFile("config"); + String include = "[include]\npath=" + includedPath + "\n"; + Files.write(config.toPath(), include.getBytes()); + FileBasedConfig fbConfig = new FileBasedConfig(null, config, + FS.DETECTED); + try { + fbConfig.load(); + fail("Expected ConfigInvalidException"); + } catch (ConfigInvalidException e) { + // Check that there is some exception in the chain that contains + // includedPath + for (Throwable t = e; t != null; t = t.getCause()) { + if (t.getMessage().contains(includedPath)) { + return; + } + } + fail("Expected to find the path in the exception message: " + + includedPath); + } + } + 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 74be24665..c43411779 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -368,6 +368,7 @@ invalidIgnoreRule=Exception caught while parsing ignore rule ''{0}''. invalidIntegerValue=Invalid integer value: {0}.{1}={2} invalidKey=Invalid key: {0} invalidLineInConfigFile=Invalid line in config file +invalidLineInConfigFileWithParam=Invalid line in config file: {0} invalidModeFor=Invalid mode {0} for {1} {2} in {3}. invalidModeForPath=Invalid mode {0} for path {1} invalidObject=Invalid {0} {1}: {2} 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 7f91b30a9..9739672f8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -429,6 +429,7 @@ public class JGitText extends TranslationBundle { /***/ public String invalidIntegerValue; /***/ public String invalidKey; /***/ public String invalidLineInConfigFile; + /***/ public String invalidLineInConfigFileWithParam; /***/ public String invalidModeFor; /***/ public String invalidModeForPath; /***/ public String invalidObject; 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 a6313f0cc..4d558c9fc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java @@ -1125,7 +1125,7 @@ public class Config { } else e.value = readValue(in); - if (e.section.equals("include")) { //$NON-NLS-1$ + if (e.section.equalsIgnoreCase("include")) { //$NON-NLS-1$ addIncludedConfig(newEntries, e, depth); } } else @@ -1154,10 +1154,10 @@ public class Config { private void addIncludedConfig(final List newEntries, ConfigLine line, int depth) throws ConfigInvalidException { - if (!line.name.equals("path") || //$NON-NLS-1$ + if (!line.name.equalsIgnoreCase("path") || //$NON-NLS-1$ line.value == null || line.value.equals(MAGIC_EMPTY_VALUE)) { - throw new ConfigInvalidException( - JGitText.get().invalidLineInConfigFile); + throw new ConfigInvalidException(MessageFormat.format( + JGitText.get().invalidLineInConfigFileWithParam, line)); } byte[] bytes = readIncludedConfig(line.value); if (bytes == null) { @@ -1171,7 +1171,12 @@ public class Config { } else { decoded = RawParseUtils.decode(bytes); } - newEntries.addAll(fromTextRecurse(decoded, depth + 1)); + try { + newEntries.addAll(fromTextRecurse(decoded, depth + 1)); + } catch (ConfigInvalidException e) { + throw new ConfigInvalidException(MessageFormat + .format(JGitText.get().cannotReadFile, line.value), e); + } } private ConfigSnapshot newState() {