Disallow explicit signs in java.sql.Timestamp#valueOf()

The implementation was cleaned up:
- removed useless checks and improved a few other
- fixed variable names
- fixed badTimestampString

Bug: 5239391
Change-Id: I51ea2ba848b0c2d2d2f26807a1c56e7e015ef9f4

3 files changed, 168 insertions, 90 deletions

diff --git a/luni/src/main/java/java/sql/Timestamp.java b/luni/src/main/java/java/sql/Timestamp.java
index f35fa9b..a45a2b8 100644
--- a/luni/src/main/java/java/sql/Timestamp.java
+++ b/luni/src/main/java/java/sql/Timestamp.java
@@ -408,7 +408,7 @@ public class Timestamp extends Date {
             throw new IllegalArgumentException("Argument cannot be null");
         }
 
-        // omit trailing whitespace
+        // Omit trailing whitespace
         s = s.trim();
         if (!Pattern.matches(TIME_FORMAT_REGEX, s)) {
             throw badTimestampString(s);
@@ -424,14 +424,14 @@ public class Timestamp extends Date {
          * with the ParsePosition indicating the index of the "." which should
          * precede the nanoseconds value
          */
-        Date theDate;
+        Date date;
         try {
-            theDate = df.parse(s, pp);
+            date = df.parse(s, pp);
         } catch (Exception e) {
             throw badTimestampString(s);
         }
 
-        if (theDate == null) {
+        if (date == null) {
             throw badTimestampString(s);
         }
 
@@ -445,66 +445,38 @@ public class Timestamp extends Date {
          */
         int position = pp.getIndex();
         int remaining = s.length() - position;
-        int theNanos;
+        int nanos;
 
         if (remaining == 0) {
             // First, allow for the case where no fraction of a second is given:
-            theNanos = 0;
+            nanos = 0;
         } else {
-            /*
-             * Case where fraction of a second is specified: Require 1 character
-             * plus the "." in the remaining part of the string...
-             */
-            if ((s.length() - position) < ".n".length()) {
+            // Validate the string is in the range ".0" to ".999999999"
+            if (remaining < 2 || remaining > 10 || s.charAt(position) != '.') {
                 throw badTimestampString(s);
             }
-
-            /*
-             * If we're strict, we should not allow any EXTRA characters after
-             * the 9 digits
-             */
-            if ((s.length() - position) > ".nnnnnnnnn".length()) {
-                throw badTimestampString(s);
-            }
-
-            // Require the next character to be a "."
-            if (s.charAt(position) != '.') {
-                throw new NumberFormatException("Bad input string format: expected '.' not '" +
-                        s.charAt(position) + "' in \"" + s + "\"");
-            }
-            // Get the length of the number string - need to account for the '.'
-            int nanoLength = s.length() - position - 1;
-
-            // Get the 9 characters following the "." as an integer
-            String theNanoString = s.substring(position + 1, position + 1
-                    + nanoLength);
-            /*
-             * We must adjust for the cases where the nanos String was not 9
-             * characters long by padding out with zeros
-             */
-            theNanoString = theNanoString + "000000000";
-            theNanoString = theNanoString.substring(0, 9);
-
             try {
-                theNanos = Integer.parseInt(theNanoString);
-            } catch (Exception e) {
-                // If we get here, the string was not a number
+                nanos = Integer.parsePositiveInt(s.substring(position + 1));
+            } catch (NumberFormatException e) {
                 throw badTimestampString(s);
             }
+            // We must adjust for the cases where the nanos String was not 9
+            // characters long (i.e. ".123" means 123000000 nanos)
+            if (nanos != 0) {
+                for (int i = remaining - 1; i < 9; i++) {
+                    nanos *= 10;
+                }
+            }
         }
 
-        if (theNanos < 0 || theNanos > 999999999) {
-            throw badTimestampString(s);
-        }
-
-        Timestamp theTimestamp = new Timestamp(theDate.getTime());
-        theTimestamp.setNanos(theNanos);
+        Timestamp timestamp = new Timestamp(date.getTime());
+        timestamp.setNanos(nanos);
 
-        return theTimestamp;
+        return timestamp;
     }
 
     private static IllegalArgumentException badTimestampString(String s) {
-        throw new IllegalArgumentException("Timestamp format must be " +
+        return new IllegalArgumentException("Timestamp format must be " +
                 "yyyy-MM-dd HH:mm:ss.fffffffff; was '" + s + "'");
     }
 }
diff --git a/luni/src/test/java/libcore/java/sql/OldTimestampTest.java b/luni/src/test/java/libcore/java/sql/OldTimestampTest.java
deleted file mode 100644
index ab2034b..0000000
--- a/luni/src/test/java/libcore/java/sql/OldTimestampTest.java
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package libcore.java.sql;
-
-import java.sql.Timestamp;
-import java.util.TimeZone;
-import junit.framework.TestCase;
-
-public final class OldTimestampTest extends TestCase {
-
-    public void test_toString() {
-        TimeZone.setDefault(TimeZone.getTimeZone("UTC"));
-
-        Timestamp t1 = new Timestamp(Long.MIN_VALUE);
-        assertEquals("292278994-08-17 07:12:55.192", t1.toString());
-
-        Timestamp t2 = new Timestamp(Long.MIN_VALUE + 1);
-        assertEquals("292278994-08-17 07:12:55.193", t2.toString());
-
-        Timestamp t3 = new Timestamp(Long.MIN_VALUE + 807);
-        assertEquals("292278994-08-17 07:12:55.999", t3.toString());
-
-        Timestamp t4 = new Timestamp(Long.MIN_VALUE + 808);
-        assertEquals("292269055-12-02 16:47:05.0", t4.toString());
-    }
-}
diff --git a/luni/src/test/java/libcore/java/sql/TimestampTest.java b/luni/src/test/java/libcore/java/sql/TimestampTest.java
new file mode 100644
index 0000000..2985848
--- /dev/null
+++ b/luni/src/test/java/libcore/java/sql/TimestampTest.java
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package libcore.java.sql;
+
+import java.sql.Timestamp;
+import java.util.TimeZone;
+import junit.framework.TestCase;
+
+public final class TimestampTest extends TestCase {
+
+  public void testToString() {
+    // Timestamp uses the current default timezone in toString() to convert to
+    // human-readable strings.
+    TimeZone defaultTimeZone = TimeZone.getDefault();
+    TimeZone.setDefault(TimeZone.getTimeZone("UTC"));
+    try {
+      Timestamp t1 = new Timestamp(Long.MIN_VALUE);
+      assertEquals("292278994-08-17 07:12:55.192", t1.toString());
+
+      Timestamp t2 = new Timestamp(Long.MIN_VALUE + 1);
+      assertEquals("292278994-08-17 07:12:55.193", t2.toString());
+
+      Timestamp t3 = new Timestamp(Long.MIN_VALUE + 807);
+      assertEquals("292278994-08-17 07:12:55.999", t3.toString());
+
+      Timestamp t4 = new Timestamp(Long.MIN_VALUE + 808);
+      assertEquals("292269055-12-02 16:47:05.0", t4.toString());
+    } finally {
+      TimeZone.setDefault(defaultTimeZone);
+    }
+  }
+
+  public void testValueOf() {
+    // Timestamp uses the current default timezone in valueOf(String) to convert
+    // from human-readable strings.
+    TimeZone defaultTimeZone = TimeZone.getDefault();
+    TimeZone.setDefault(TimeZone.getTimeZone("GMT"));
+    try {
+      Timestamp t1 = Timestamp.valueOf("2001-12-31 21:45:57.123456789");
+      assertEquals(1009835157000L + 123456789 / 1000000, t1.getTime());
+      assertEquals(123456789, t1.getNanos());
+
+      Timestamp t2 = Timestamp.valueOf("2001-01-02 01:05:07.123");
+      assertEquals(978397507000L + 123000000 / 1000000, t2.getTime());
+      assertEquals(123000000, t2.getNanos());
+
+      Timestamp t3 = Timestamp.valueOf("2001-01-02 01:05:07");
+      assertEquals(978397507000L, t3.getTime());
+      assertEquals(0, t3.getNanos());
+    } finally {
+      TimeZone.setDefault(defaultTimeZone);
+    }
+  }
+
+  public void testValueOfInvalid() {
+    try {
+      Timestamp.valueOf("");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("+2001-12-31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001-+12-31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001-12-+31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("-2001-12-31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001--12-31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001-12--31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001--");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001--31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("-12-31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("-12-");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("--31");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001-12-31 21:45:57.+12345678");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001-12-31 21:45:57.-12345678");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+
+    try {
+      Timestamp.valueOf("2001-12-31 21:45:57.1234567891");
+      fail();
+    } catch (IllegalArgumentException expected) { }
+  }
+
+}