From 0085f48710cdcf13baec5dd7b48b138776efac29 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Wed, 23 Aug 2017 14:34:18 -0400
Subject: [PATCH] Support labels >33 chars in LabelVote#parseWithEquals

In 57ecf24c back in 2013, I accidentally wrote this:
  Short.parseShort(text.substring(e + 1), text.length())
where *obviously* I meant this:
  Short.parseShort(text.substring(e + 1, text.length()))

This called Short#parseShort(String, int), where the second arg is a
radix. (The second arg to substring is also not necessary in this case,
but would have been harmlessly correct if it were present.)

It's surprising and lucky that passing an arbitrary radix managed to
have correct behavior in practically all cases, but this was in fact the
case due a combination of two facts:
  * text.length() is always at least 3 ("X=0"), usually longer, and
    only exceeds Character.MAX_RADIX (36) when the label name exceeds 33
    characters.
  * The value itself is rarely greater than 2 or 3, essentially never
    requiring multiple digits (unless someone were foolhardy enough to
    use a very large range), so any radix greater than or equal to the
    value would result in parsing the value correctly.

As an amusing side effect, the old code would parse "Code-Review=c"
"correctly," but would fail on "Code-Review=d".

Fix this bug and increase test coverage.

Change-Id: I17f4fb963e1e0d7b7bd51d689e7557cbcfc8b474
---
 .../google/gerrit/server/util/LabelVote.java  |  2 +-
 .../gerrit/server/util/LabelVoteTest.java     | 30 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
index b5e180df36..a840e87db9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java
@@ -52,7 +52,7 @@ public abstract class LabelVote {
     checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote");
     int e = text.lastIndexOf('=');
     checkArgument(e >= 0, "Label vote missing '=': %s", text);
-    return create(text.substring(0, e), Short.parseShort(text.substring(e + 1), text.length()));
+    return create(text.substring(0, e), Short.parseShort(text.substring(e + 1)));
   }
 
   public static StringBuilder appendTo(StringBuilder sb, String label, short value) {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/util/LabelVoteTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/util/LabelVoteTest.java
index 0592041c8f..d3cb927ab0 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/util/LabelVoteTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/util/LabelVoteTest.java
@@ -15,6 +15,8 @@
 package com.google.gerrit.server.util;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import org.junit.Test;
 
@@ -75,6 +77,25 @@ public class LabelVoteTest {
     l = LabelVote.parseWithEquals("Code-Review=+2");
     assertEquals("Code-Review", l.label());
     assertEquals((short) 2, l.value());
+    l = LabelVote.parseWithEquals("R=0");
+    assertEquals("R", l.label());
+    assertEquals((short) 0, l.value());
+
+    String longName = "A-loooooooooooooooooooooooooooooooooooooooooooooooooong-label";
+    // Regression test: an old bug passed the string length as a radix to Short#parseShort.
+    assertTrue(longName.length() > Character.MAX_RADIX);
+    l = LabelVote.parseWithEquals(longName + "=+1");
+    assertEquals(longName, l.label());
+    assertEquals((short) 1, l.value());
+
+    assertParseWithEqualsFails(null);
+    assertParseWithEqualsFails("");
+    assertParseWithEqualsFails("Code-Review");
+    assertParseWithEqualsFails("=1");
+    assertParseWithEqualsFails("=.1");
+    assertParseWithEqualsFails("=a1");
+    assertParseWithEqualsFails("=1a");
+    assertParseWithEqualsFails("=.");
   }
 
   @Test
@@ -85,4 +106,13 @@ public class LabelVoteTest {
     assertEquals("Code-Review=+1", LabelVote.parseWithEquals("Code-Review=+1").formatWithEquals());
     assertEquals("Code-Review=+2", LabelVote.parseWithEquals("Code-Review=+2").formatWithEquals());
   }
+
+  private void assertParseWithEqualsFails(String value) {
+    try {
+      LabelVote.parseWithEquals(value);
+      fail("expected IllegalArgumentException when parsing \"" + value + "\"");
+    } catch (IllegalArgumentException e) {
+      // Expected.
+    }
+  }
 }