[JGIT PATCH 1/2] Add support for boolean config values "yes", "no"

Subsystems: the rest

DORMANTno replies

6 messages, 4 authors, 2016-06-15 · open the first message on its own page

[JGIT PATCH 1/2] Add support for boolean config values "yes", "no"

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:46:43

In 8f8c6fafd92f (shipped in 1.6.3) Linus taught C Git how to read
boolean configuration values set to "yes" as true and "no" as false.
Add support for these values, and some test cases.

Signed-off-by: Shawn O. Pearce <redacted>
---

 This little 2 patch series is to adjust some cases in JGit to
 match Git 1.6.3 semantics.

 .../org/spearce/jgit/lib/RepositoryConfigTest.java |   70 ++++++++++++++++++--
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |   13 +++-
 2 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
index 4b5314c..ed573e1 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
@@ -57,9 +57,7 @@
 	 * @throws IOException
 	 */
 	public void test001_ReadBareKey() throws IOException {
-		final File path = writeTrashFile("config_001", "[foo]\nbar\n");
-		RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
-		System.out.println(repositoryConfig.getString("foo", null, "bar"));
+		final RepositoryConfig repositoryConfig = read("[foo]\nbar\n");
 		assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
 		assertEquals("", repositoryConfig.getString("foo", null, "bar"));
 	}
@@ -70,8 +68,7 @@ public void test001_ReadBareKey() throws IOException {
 	 * @throws IOException
 	 */
 	public void test002_ReadWithSubsection() throws IOException {
-		final File path = writeTrashFile("config_002", "[foo \"zip\"]\nbar\n[foo \"zap\"]\nbar=false\nn=3\n");
-		RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
+		final RepositoryConfig repositoryConfig = read("[foo \"zip\"]\nbar\n[foo \"zap\"]\nbar=false\nn=3\n");
 		assertEquals(true, repositoryConfig.getBoolean("foo", "zip", "bar", false));
 		assertEquals("", repositoryConfig.getString("foo","zip", "bar"));
 		assertEquals(false, repositoryConfig.getBoolean("foo", "zap", "bar", true));
@@ -113,8 +110,7 @@ assertTrue(Arrays.equals(values.toArray(), repositoryConfig
 	}
 
 	public void test006_readCaseInsensitive() throws IOException {
-		final File path = writeTrashFile("config_001", "[Foo]\nBar\n");
-		RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
+		final RepositoryConfig repositoryConfig = read("[Foo]\nBar\n");
 		assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
 		assertEquals("", repositoryConfig.getString("foo", null, "bar"));
 	}
@@ -182,4 +178,64 @@ public void test007_readUserInfos() throws IOException {
 		assertEquals("local username", authorName);
 		assertEquals("author@localemail", authorEmail);
 	}
+
+	public void testReadBoolean_TrueFalse1() throws IOException {
+		final RepositoryConfig c = read("[s]\na = true\nb = false\n");
+		assertEquals("true", c.getString("s", null, "a"));
+		assertEquals("false", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_TrueFalse2() throws IOException {
+		final RepositoryConfig c = read("[s]\na = TrUe\nb = fAlSe\n");
+		assertEquals("TrUe", c.getString("s", null, "a"));
+		assertEquals("fAlSe", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_YesNo1() throws IOException {
+		final RepositoryConfig c = read("[s]\na = yes\nb = no\n");
+		assertEquals("yes", c.getString("s", null, "a"));
+		assertEquals("no", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_YesNo2() throws IOException {
+		final RepositoryConfig c = read("[s]\na = yEs\nb = NO\n");
+		assertEquals("yEs", c.getString("s", null, "a"));
+		assertEquals("NO", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_OnOff1() throws IOException {
+		final RepositoryConfig c = read("[s]\na = on\nb = off\n");
+		assertEquals("on", c.getString("s", null, "a"));
+		assertEquals("off", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_OnOff2() throws IOException {
+		final RepositoryConfig c = read("[s]\na = ON\nb = OFF\n");
+		assertEquals("ON", c.getString("s", null, "a"));
+		assertEquals("OFF", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	private RepositoryConfig read(final String content) throws IOException {
+		final File p = writeTrashFile(getName() + ".config", content);
+		final RepositoryConfig c = new RepositoryConfig(null, p);
+		return c;
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
index cb287ee..e3a303f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -254,10 +254,19 @@ public boolean getBoolean(final String section, String subsection,
 			return defaultValue;
 
 		n = n.toLowerCase();
-		if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+		if (MAGIC_EMPTY_VALUE.equals(n)
+				|| "yes".equalsIgnoreCase(n)
+				|| "true".equalsIgnoreCase(n)
+				|| "1".equals(n)
+				|| "on".equals(n)) {
 			return true;
-		} else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+
+		} else if ("no".equalsIgnoreCase(n)
+				|| "false".equalsIgnoreCase(n)
+				|| "0".equalsIgnoreCase(n)
+				|| "off".equalsIgnoreCase(n)) {
 			return false;
+
 		} else {
 			throw new IllegalArgumentException("Invalid boolean value: "
 					+ section + "." + name + "=" + n);
-- 
1.6.3.195.gad81

[JGIT PATCH 2/2] Make Repository.isValidRefName compatible with Git 1.6.3

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:46:43

In 3e262b95c509 I taught C Git to disallow refs whose names end in
".lock".  This suffix is used by the atomic update mechanism as a
signal that the ref is being modified.  When reading a loose ref
directory both JGit and C Git skip over any files whose names end
with this suffix, as the file is assumed to be one of these magic
locking files from another concurrent process.  Consequently, any
ref that ends with this name will become invisible once created.

We also add a suite of tests for the isValidRefName function, fix
its formatting to better conform to current style conventions, and
correct the result for "master"; this is not a valid ref name as it
has only 1 path component.  At least 2 path components is required.

Signed-off-by: Shawn O. Pearce <redacted>
---
 .../tst/org/spearce/jgit/lib/ValidRefNameTest.java |  168 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/Repository.java       |   28 ++--
 2 files changed, 181 insertions(+), 15 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ValidRefNameTest.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ValidRefNameTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ValidRefNameTest.java
new file mode 100644
index 0000000..5cca682
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ValidRefNameTest.java
@@ -0,0 +1,168 @@
+/*
+ * Copyright (C) 2009, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.lib;
+
+import junit.framework.TestCase;
+
+public class ValidRefNameTest extends TestCase {
+	private static void assertValid(final boolean exp, final String name) {
+		assertEquals("\"" + name + "\"", exp, Repository.isValidRefName(name));
+	}
+
+	public void testEmptyString() {
+		assertValid(false, "");
+		assertValid(false, "/");
+	}
+
+	public void testMustHaveTwoComponents() {
+		assertValid(false, "master");
+		assertValid(true, "heads/master");
+	}
+
+	public void testValidHead() {
+		assertValid(true, "refs/heads/master");
+		assertValid(true, "refs/heads/pu");
+		assertValid(true, "refs/heads/z");
+		assertValid(true, "refs/heads/FoO");
+	}
+
+	public void testValidTag() {
+		assertValid(true, "refs/tags/v1.0");
+	}
+
+	public void testNoLockSuffix() {
+		assertValid(false, "refs/heads/master.lock");
+	}
+
+	public void testNoDirectorySuffix() {
+		assertValid(false, "refs/heads/master/");
+	}
+
+	public void testNoSpace() {
+		assertValid(false, "refs/heads/i haz space");
+	}
+
+	public void testNoAsciiControlCharacters() {
+		for (char c = '\0'; c < ' '; c++)
+			assertValid(false, "refs/heads/mast" + c + "er");
+	}
+
+	public void testNoBareDot() {
+		assertValid(false, "refs/heads/.");
+		assertValid(false, "refs/heads/..");
+		assertValid(false, "refs/heads/./master");
+		assertValid(false, "refs/heads/../master");
+	}
+
+	public void testNoLeadingDot() {
+		assertValid(false, ".");
+		assertValid(false, "refs/heads/.bar");
+		assertValid(false, "refs/heads/..bar");
+	}
+
+	public void testContainsDot() {
+		assertValid(true, "refs/heads/m.a.s.t.e.r");
+		assertValid(false, "refs/heads/master..pu");
+	}
+
+	public void testNoMagicRefCharacters() {
+		assertValid(false, "refs/heads/master^");
+		assertValid(false, "refs/heads/^master");
+		assertValid(false, "^refs/heads/master");
+
+		assertValid(false, "refs/heads/master~");
+		assertValid(false, "refs/heads/~master");
+		assertValid(false, "~refs/heads/master");
+
+		assertValid(false, "refs/heads/master:");
+		assertValid(false, "refs/heads/:master");
+		assertValid(false, ":refs/heads/master");
+	}
+
+	public void testShellGlob() {
+		assertValid(false, "refs/heads/master?");
+		assertValid(false, "refs/heads/?master");
+		assertValid(false, "?refs/heads/master");
+
+		assertValid(false, "refs/heads/master[");
+		assertValid(false, "refs/heads/[master");
+		assertValid(false, "[refs/heads/master");
+
+		assertValid(false, "refs/heads/master*");
+		assertValid(false, "refs/heads/*master");
+		assertValid(false, "*refs/heads/master");
+	}
+
+	public void testValidSpecialCharacters() {
+		assertValid(true, "refs/heads/!");
+		assertValid(true, "refs/heads/\"");
+		assertValid(true, "refs/heads/#");
+		assertValid(true, "refs/heads/$");
+		assertValid(true, "refs/heads/%");
+		assertValid(true, "refs/heads/&");
+		assertValid(true, "refs/heads/'");
+		assertValid(true, "refs/heads/(");
+		assertValid(true, "refs/heads/)");
+		assertValid(true, "refs/heads/+");
+		assertValid(true, "refs/heads/,");
+		assertValid(true, "refs/heads/-");
+		assertValid(true, "refs/heads/;");
+		assertValid(true, "refs/heads/<");
+		assertValid(true, "refs/heads/=");
+		assertValid(true, "refs/heads/>");
+		assertValid(true, "refs/heads/@");
+		assertValid(true, "refs/heads/]");
+		assertValid(true, "refs/heads/_");
+		assertValid(true, "refs/heads/`");
+		assertValid(true, "refs/heads/{");
+		assertValid(true, "refs/heads/|");
+		assertValid(true, "refs/heads/}");
+
+		// This is valid on UNIX, but not on Windows.
+		//
+		assertValid(true, "refs/heads/\\");
+	}
+
+	public void testRefLogQueryIsValidRef() {
+		// Yes, these are actually ambiguous. You can create a ref
+		// that also looks like a reflog query.
+		//
+		assertValid(true, "refs/heads/master@{1}");
+		assertValid(true, "refs/heads/master@{1.hour.ago}");
+	}
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 5baa7a0..66dcd46 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -939,36 +939,34 @@ public static boolean isValidRefName(final String refName) {
 		final int len = refName.length();
 		if (len == 0)
 			return false;
+		if (refName.endsWith(".lock"))
+			return false;
 
+		int components = 1;
 		char p = '\0';
-		for (int i=0; i<len; ++i) {
-			char c = refName.charAt(i);
+		for (int i = 0; i < len; i++) {
+			final char c = refName.charAt(i);
 			if (c <= ' ')
 				return false;
-			switch(c) {
+			switch (c) {
 			case '.':
-				if (i == 0)
-					return false;
-				if (p == '/')
-					return false;
-				if (p == '.')
+				switch (p) {
+				case '\0': case '/': case '.':
 					return false;
+				}
 				break;
 			case '/':
-				if (i == 0)
-					return false;
-				if (i == len -1)
+				if (i == 0 || i == len - 1)
 					return false;
+				components++;
 				break;
 			case '~': case '^': case ':':
-			case '?': case '[':
-				return false;
-			case '*':
+			case '?': case '[': case '*':
 				return false;
 			}
 			p = c;
 		}
-		return true;
+		return components > 1;
 	}
 
 	/**
-- 
1.6.3.195.gad81

Re: [JGIT PATCH 1/2] Add support for boolean config values "yes", "no"

From: Brandon Casey <hidden>
Date: 2016-06-15 22:46:43

Shawn O. Pearce wrote:
In 8f8c6fafd92f (shipped in 1.6.3) Linus taught C Git how to read
boolean configuration values set to "yes" as true and "no" as false.
Add support for these values, and some test cases.
Linus added support for "on" and "off" in that commit as it appears
your commit is also doing. :)

I was about to point out that you are not ignoring case for "on", but
using equalsIgnoreCase() appears to be unnecessary since above that
you are already doing n = n.toLowerCase().

-brandon

quoted hunk
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
index cb287ee..e3a303f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -254,10 +254,19 @@ public boolean getBoolean(final String section, String subsection,
 			return defaultValue;
 
 		n = n.toLowerCase();
-		if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+		if (MAGIC_EMPTY_VALUE.equals(n)
+				|| "yes".equalsIgnoreCase(n)
+				|| "true".equalsIgnoreCase(n)
+				|| "1".equals(n)
+				|| "on".equals(n)) {
 			return true;
-		} else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+
+		} else if ("no".equalsIgnoreCase(n)
+				|| "false".equalsIgnoreCase(n)
+				|| "0".equalsIgnoreCase(n)
+				|| "off".equalsIgnoreCase(n)) {
 			return false;
+
 		} else {
 			throw new IllegalArgumentException("Invalid boolean value: "
 					+ section + "." + name + "=" + n);

[JGIT PATCH v2 1/2] Add support for boolean config values "on", "off"

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:46:43

In 8f8c6fafd92f (shipped in 1.6.3) Linus taught C Git how to read
boolean configuration values set to "on" as true and "off" as false.
Add support for these values, and some test cases.

Signed-off-by: Shawn O. Pearce <redacted>
---
  Brandon Casey [off-list ref] wrote:
  > Shawn O. Pearce wrote:
  > > In 8f8c6fafd92f (shipped in 1.6.3) Linus taught C Git how to read
  > > boolean configuration values set to "yes" as true and "no" as false.
  > > Add support for these values, and some test cases.
  > 
  > Linus added support for "on" and "off" in that commit as it appears
  > your commit is also doing. :)

  Whoops, good catch, thanks.

  > I was about to point out that you are not ignoring case for "on", but
  > using equalsIgnoreCase() appears to be unnecessary since above that
  > you are already doing n = n.toLowerCase().

  I dropped the toLowerCase and used equalsIgnoreCase instead.
  There is no reason to be allocating a copy of the string when
  equalsIgnorCase is sufficient.
  
 .../org/spearce/jgit/lib/RepositoryConfigTest.java |   70 ++++++++++++++++++--
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |   14 +++-
 2 files changed, 74 insertions(+), 10 deletions(-)
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
index 4b5314c..ed573e1 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
@@ -57,9 +57,7 @@
 	 * @throws IOException
 	 */
 	public void test001_ReadBareKey() throws IOException {
-		final File path = writeTrashFile("config_001", "[foo]\nbar\n");
-		RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
-		System.out.println(repositoryConfig.getString("foo", null, "bar"));
+		final RepositoryConfig repositoryConfig = read("[foo]\nbar\n");
 		assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
 		assertEquals("", repositoryConfig.getString("foo", null, "bar"));
 	}
@@ -70,8 +68,7 @@ public void test001_ReadBareKey() throws IOException {
 	 * @throws IOException
 	 */
 	public void test002_ReadWithSubsection() throws IOException {
-		final File path = writeTrashFile("config_002", "[foo \"zip\"]\nbar\n[foo \"zap\"]\nbar=false\nn=3\n");
-		RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
+		final RepositoryConfig repositoryConfig = read("[foo \"zip\"]\nbar\n[foo \"zap\"]\nbar=false\nn=3\n");
 		assertEquals(true, repositoryConfig.getBoolean("foo", "zip", "bar", false));
 		assertEquals("", repositoryConfig.getString("foo","zip", "bar"));
 		assertEquals(false, repositoryConfig.getBoolean("foo", "zap", "bar", true));
@@ -113,8 +110,7 @@ assertTrue(Arrays.equals(values.toArray(), repositoryConfig
 	}
 
 	public void test006_readCaseInsensitive() throws IOException {
-		final File path = writeTrashFile("config_001", "[Foo]\nBar\n");
-		RepositoryConfig repositoryConfig = new RepositoryConfig(null, path);
+		final RepositoryConfig repositoryConfig = read("[Foo]\nBar\n");
 		assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false));
 		assertEquals("", repositoryConfig.getString("foo", null, "bar"));
 	}
@@ -182,4 +178,64 @@ public void test007_readUserInfos() throws IOException {
 		assertEquals("local username", authorName);
 		assertEquals("author@localemail", authorEmail);
 	}
+
+	public void testReadBoolean_TrueFalse1() throws IOException {
+		final RepositoryConfig c = read("[s]\na = true\nb = false\n");
+		assertEquals("true", c.getString("s", null, "a"));
+		assertEquals("false", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_TrueFalse2() throws IOException {
+		final RepositoryConfig c = read("[s]\na = TrUe\nb = fAlSe\n");
+		assertEquals("TrUe", c.getString("s", null, "a"));
+		assertEquals("fAlSe", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_YesNo1() throws IOException {
+		final RepositoryConfig c = read("[s]\na = yes\nb = no\n");
+		assertEquals("yes", c.getString("s", null, "a"));
+		assertEquals("no", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_YesNo2() throws IOException {
+		final RepositoryConfig c = read("[s]\na = yEs\nb = NO\n");
+		assertEquals("yEs", c.getString("s", null, "a"));
+		assertEquals("NO", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_OnOff1() throws IOException {
+		final RepositoryConfig c = read("[s]\na = on\nb = off\n");
+		assertEquals("on", c.getString("s", null, "a"));
+		assertEquals("off", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	public void testReadBoolean_OnOff2() throws IOException {
+		final RepositoryConfig c = read("[s]\na = ON\nb = OFF\n");
+		assertEquals("ON", c.getString("s", null, "a"));
+		assertEquals("OFF", c.getString("s", null, "b"));
+
+		assertTrue(c.getBoolean("s", "a", false));
+		assertFalse(c.getBoolean("s", "b", true));
+	}
+
+	private RepositoryConfig read(final String content) throws IOException {
+		final File p = writeTrashFile(getName() + ".config", content);
+		final RepositoryConfig c = new RepositoryConfig(null, p);
+		return c;
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
index cb287ee..b816604 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -253,11 +253,19 @@ public boolean getBoolean(final String section, String subsection,
 		if (n == null)
 			return defaultValue;
 
-		n = n.toLowerCase();
-		if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n) || "true".equalsIgnoreCase(n) || "1".equals(n)) {
+		if (MAGIC_EMPTY_VALUE.equals(n)
+				|| "yes".equalsIgnoreCase(n)
+				|| "true".equalsIgnoreCase(n)
+				|| "1".equals(n)
+				|| "on".equalsIgnoreCase(n)) {
 			return true;
-		} else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n) || "0".equalsIgnoreCase(n)) {
+
+		} else if ("no".equalsIgnoreCase(n)
+				|| "false".equalsIgnoreCase(n)
+				|| "0".equals(n)
+				|| "off".equalsIgnoreCase(n)) {
 			return false;
+
 		} else {
 			throw new IllegalArgumentException("Invalid boolean value: "
 					+ section + "." + name + "=" + n);
-- 
1.6.3.195.gad81

Re: [JGIT PATCH 2/2] Make Repository.isValidRefName compatible with Git 1.6.3

From: Robin Rosenberg <hidden>
Date: 2016-06-15 22:46:43

NAK, ammended patch follows.

Disallows @{, names that end with '.', '\' (that's the "almost" part) plus explicitly
allows unicode names.

-- robin
From 0854d96e2774bd084cc673ab16cf61c489f0562e Mon Sep 17 00:00:00 2001
From: Shawn O. Pearce <redacted>
Date: Thu, 7 May 2009 08:05:14 -0700
Subject: [EGIT PATCH] Make Repository.isValidRefName almost compatible with Git 1.6.3

In 3e262b95c509 I taught C Git to disallow refs whose names end in
".lock".  This suffix is used by the atomic update mechanism as a
signal that the ref is being modified.  When reading a loose ref
directory both JGit and C Git skip over any files whose names end
with this suffix, as the file is assumed to be one of these magic
locking files from another concurrent process.  Consequently, any
ref that ends with this name will become invisible once created.

In cbdffe4093be Junio disallows names that looks like reflog queries
as well as names that ends with periods.

We also add a suite of tests for the isValidRefName function, fix
its formatting to better conform to current style conventions, and
correct the result for "master"; this is not a valid ref name as it
has only 1 path component.  At least 2 path components is required.

In addition to 1.6.3 we disallow names with a '\' (backslash) since
that is a directory separator in Windows. Allowing JGit users to
create such names is just asking for trouble. This does not necessarily
prevent JGit from working with such refs in other situations.

Signed-off-by: Shawn O. Pearce <redacted>
Signed-off-by: Robin Rosenberg <redacted>
---
 .../tst/org/spearce/jgit/lib/ValidRefNameTest.java |  171 ++++++++++++++++++++
 .../src/org/spearce/jgit/lib/Repository.java       |   33 +++--
 2 files changed, 191 insertions(+), 13 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ValidRefNameTest.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ValidRefNameTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ValidRefNameTest.java
new file mode 100644
index 0000000..cb6b9a7
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ValidRefNameTest.java
@@ -0,0 +1,171 @@
+/*
+ * Copyright (C) 2009, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.lib;
+
+import junit.framework.TestCase;
+
+public class ValidRefNameTest extends TestCase {
+	private static void assertValid(final boolean exp, final String name) {
+		assertEquals("\"" + name + "\"", exp, Repository.isValidRefName(name));
+	}
+
+	public void testEmptyString() {
+		assertValid(false, "");
+		assertValid(false, "/");
+	}
+
+	public void testMustHaveTwoComponents() {
+		assertValid(false, "master");
+		assertValid(true, "heads/master");
+	}
+
+	public void testValidHead() {
+		assertValid(true, "refs/heads/master");
+		assertValid(true, "refs/heads/pu");
+		assertValid(true, "refs/heads/z");
+		assertValid(true, "refs/heads/FoO");
+	}
+
+	public void testValidTag() {
+		assertValid(true, "refs/tags/v1.0");
+	}
+
+	public void testNoLockSuffix() {
+		assertValid(false, "refs/heads/master.lock");
+	}
+
+	public void testNoDirectorySuffix() {
+		assertValid(false, "refs/heads/master/");
+	}
+
+	public void testNoSpace() {
+		assertValid(false, "refs/heads/i haz space");
+	}
+
+	public void testNoAsciiControlCharacters() {
+		for (char c = '\0'; c < ' '; c++)
+			assertValid(false, "refs/heads/mast" + c + "er");
+	}
+
+	public void testNoBareDot() {
+		assertValid(false, "refs/heads/.");
+		assertValid(false, "refs/heads/..");
+		assertValid(false, "refs/heads/./master");
+		assertValid(false, "refs/heads/../master");
+	}
+
+	public void testNoLeadingOrTrailingDot() {
+		assertValid(false, ".");
+		assertValid(false, "refs/heads/.bar");
+		assertValid(false, "refs/heads/..bar");
+		assertValid(false, "refs/heads/bar.");
+	}
+
+	public void testContainsDot() {
+		assertValid(true, "refs/heads/m.a.s.t.e.r");
+		assertValid(false, "refs/heads/master..pu");
+	}
+
+	public void testNoMagicRefCharacters() {
+		assertValid(false, "refs/heads/master^");
+		assertValid(false, "refs/heads/^master");
+		assertValid(false, "^refs/heads/master");
+
+		assertValid(false, "refs/heads/master~");
+		assertValid(false, "refs/heads/~master");
+		assertValid(false, "~refs/heads/master");
+
+		assertValid(false, "refs/heads/master:");
+		assertValid(false, "refs/heads/:master");
+		assertValid(false, ":refs/heads/master");
+	}
+
+	public void testShellGlob() {
+		assertValid(false, "refs/heads/master?");
+		assertValid(false, "refs/heads/?master");
+		assertValid(false, "?refs/heads/master");
+
+		assertValid(false, "refs/heads/master[");
+		assertValid(false, "refs/heads/[master");
+		assertValid(false, "[refs/heads/master");
+
+		assertValid(false, "refs/heads/master*");
+		assertValid(false, "refs/heads/*master");
+		assertValid(false, "*refs/heads/master");
+	}
+
+	public void testValidSpecialCharacters() {
+		assertValid(true, "refs/heads/!");
+		assertValid(true, "refs/heads/\"");
+		assertValid(true, "refs/heads/#");
+		assertValid(true, "refs/heads/$");
+		assertValid(true, "refs/heads/%");
+		assertValid(true, "refs/heads/&");
+		assertValid(true, "refs/heads/'");
+		assertValid(true, "refs/heads/(");
+		assertValid(true, "refs/heads/)");
+		assertValid(true, "refs/heads/+");
+		assertValid(true, "refs/heads/,");
+		assertValid(true, "refs/heads/-");
+		assertValid(true, "refs/heads/;");
+		assertValid(true, "refs/heads/<");
+		assertValid(true, "refs/heads/=");
+		assertValid(true, "refs/heads/>");
+		assertValid(true, "refs/heads/@");
+		assertValid(true, "refs/heads/]");
+		assertValid(true, "refs/heads/_");
+		assertValid(true, "refs/heads/`");
+		assertValid(true, "refs/heads/{");
+		assertValid(true, "refs/heads/|");
+		assertValid(true, "refs/heads/}");
+
+		// This is valid on UNIX, but not on Windows
+		// hence we make in invalid due to non-portability
+		//
+		assertValid(false, "refs/heads/\\");
+	}
+
+	public void testUnicodeNames() {
+		assertValid(true, "refs/heads/\u00e5ngstr\u00f6m");
+	}
+
+	public void testRefLogQueryIsValidRef() {
+		assertValid(false, "refs/heads/master@{1}");
+		assertValid(false, "refs/heads/master@{1.hour.ago}");
+	}
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 5baa7a0..a47207d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -931,6 +931,8 @@ public RepositoryState getRepositoryState() {
 	 * a special meaning in a Git object reference expression. Some other
 	 * dangerous characters are also excluded.
 	 *
+	 * For portability reasons '\' is excluded
+	 *
 	 * @param refName
 	 *
 	 * @return true if refName is a valid ref name
@@ -939,36 +941,41 @@ public static boolean isValidRefName(final String refName) {
 		final int len = refName.length();
 		if (len == 0)
 			return false;
+		if (refName.endsWith(".lock"))
+			return false;
 
+		int components = 1;
 		char p = '\0';
-		for (int i=0; i<len; ++i) {
-			char c = refName.charAt(i);
+		for (int i = 0; i < len; i++) {
+			final char c = refName.charAt(i);
 			if (c <= ' ')
 				return false;
-			switch(c) {
+			switch (c) {
 			case '.':
-				if (i == 0)
-					return false;
-				if (p == '/')
+				switch (p) {
+				case '\0': case '/': case '.':
 					return false;
-				if (p == '.')
+				}
+				if (i == len -1)
 					return false;
 				break;
 			case '/':
-				if (i == 0)
+				if (i == 0 || i == len - 1)
 					return false;
-				if (i == len -1)
+				components++;
+				break;
+			case '@':
+				if (p == '{')
 					return false;
 				break;
 			case '~': case '^': case ':':
-			case '?': case '[':
-				return false;
-			case '*':
+			case '?': case '[': case '*':
+			case '\\':
 				return false;
 			}
 			p = c;
 		}
-		return true;
+		return components > 1;
 	}
 
 	/**
-- 
1.6.3

Re: [JGIT PATCH 2/2] Make Repository.isValidRefName compatible with Git 1.6.3

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:46:43


On Fri, 8 May 2009, Robin Rosenberg wrote:
In 3e262b95c509 I taught C Git to disallow refs whose names end in
".lock".
Btw, I think we should revert that, and instead change our naming for 
lock-files.

It's a silly limitation, and for a stupid reason, I think. I can well 
imagine people using a branch naming policy of using '.' instead of 
spaces, and then It makes perfect sense to call a branch "fix.vm.lock" 
when you have a VM locking issue you want to fix.

So I think we should remove that stupid "*.lock" rule from the C version 
(and then obviously from the JGIT one too).

Now, that leaves what we _should_ call the lock-files, and the only sane 
thing to do is to pick a sequence that is already invalid for other 
reasons. We could make the lockfile end in "..lock", for example (".." is 
not legal in names due to the confusion with the "a..b" range notation), 
or just end or begin with a "." (same issue, except "..." now).

Or just make it end in '~' which is also invalid for similar reasons.

		Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help