Cleaned up conversion of int and long in converters (#551)

* Cleaned up conversion of `int` and `long` in converters
* Added tests for Integer.MAX_VALUE and MIN_VALUE

Author: chrjohn

quickfixj-core/src/main/java/quickfix/field/converter/AbstractDateTimeConverter.java

@@ -65,14 +65,6 @@ protected static void throwFieldConvertError(String value, String type)
         throw new FieldConvertError("invalid UTC " + type + " value: " + value);
     }
 
-    protected static long parseLong(String s) {
-        long n = 0;
-        for (int i = 0; i < s.length(); i++) {
-            n = (n * 10) + (s.charAt(i) - '0');
-        }
-        return n;
-    }
-
     protected DateFormat createDateFormat(String format) {
         SimpleDateFormat sdf = new SimpleDateFormat(format);
         sdf.setTimeZone(TimeZone.getTimeZone("UTC"));

quickfixj-core/src/main/java/quickfix/field/converter/IntConverter.java

@@ -27,7 +27,7 @@
 public final class IntConverter {
 
     /**
-     * Convert and integer to a String
+     * Convert an integer to a String
      *
      * @param i the integer to convert
      * @return the String representing the integer
@@ -42,7 +42,8 @@ public static String convert(int i) {
      *
      * @param value the String to convert
      * @return the converted integer
-     * @throws FieldConvertError raised if the String does not represent a valid integer
+     * @throws FieldConvertError raised if the String does not represent a valid
+     * integer
      * @see java.lang.Integer#parseInt(String)
      */
     public static int convert(String value) throws FieldConvertError {
@@ -57,4 +58,52 @@ public static int convert(String value) throws FieldConvertError {
             throw new FieldConvertError("invalid integral value: " + value + ": " + e);
         }
     }
+
+    /**
+     * Please note that input needs to be validated first, otherwise unexpected
+     * results may occur. Please also note that this method has no range or overflow
+     * check, so please only use it when you are sure that no overflow might occur
+     * (e.g. for parsing seconds or smaller integers).
+     *
+     * @param value the String to convert
+     * @param off offset position from which String should be parsed
+     * @param len length to parse
+     * @return the converted int
+     */
+    static int parseInt(String value, int off, int len) {
+        int num = 0;
+        boolean negative = false;
+        for (int index = 0; index < len; index++) {
+            final char charAt = value.charAt(off + index);
+            if (index == 0 && charAt == '-') {
+                negative = true;
+                continue;
+            }
+            num = (num * 10) + charAt - '0';
+        }
+        return negative ? -num : num;
+    }
+
+    /**
+     * Please note that input needs to be validated first, otherwise unexpected
+     * results may occur. Please also note that this method has no range or overflow
+     * check, so please only use it when you are sure that no overflow might occur
+     * (e.g. for parsing seconds or smaller integers).
+     * 
+     * @param value the String to convert
+     * @return the converted long
+     */
+    static long parseLong(String value) {
+        long num = 0;
+        boolean negative = false;
+        for (int index = 0; index < value.length(); index++) {
+            final char charAt = value.charAt(index);
+            if (index == 0 && charAt == '-') {
+                negative = true;
+                continue;
+            }
+            num = (num * 10) + (value.charAt(index) - '0');
+        }
+        return negative ? -num : num;
+    }
 }

quickfixj-core/src/main/java/quickfix/field/converter/UtcTimestampConverter.java

@@ -1,4 +1,4 @@
-/** *****************************************************************************
+/*******************************************************************************
  * Copyright (c) quickfixengine.org  All rights reserved.
  *
  * This file is part of the QuickFIX FIX Engine
@@ -15,7 +15,7 @@
  *
  * Contact ask@quickfixengine.org if any conditions of this licensing
  * are not clear to you.
- ***************************************************************************** */
+ ******************************************************************************/
 package quickfix.field.converter;
 
 import quickfix.UtcTimestampPrecision;
@@ -120,7 +120,7 @@ public static Date convert(String value) throws FieldConvertError {
         long timeOffset = getTimeOffsetSeconds(value);
         if (value.length() >= LENGTH_INCL_MILLIS) { // format has already been verified
             // accept up to picosenconds but parse only up to milliseconds
-            timeOffset += parseLong(value.substring(18, LENGTH_INCL_MILLIS));
+            timeOffset += IntConverter.parseLong(value.substring(18, LENGTH_INCL_MILLIS));
         }
         return new Date(getMillisForDay(value) + timeOffset);
     }
@@ -160,11 +160,7 @@ public static LocalDateTime convertToLocalDateTime(String value) throws FieldCon
     }
 
     private static int parseInt(String value, int off, int len) {
-        int num = 0;
-        for (int index = 0; index < len; index++) {
-            num = (num * 10) + value.charAt(off + index) - '0';
-        }
-        return num;
+        return IntConverter.parseInt(value, off, len);
     }
 
     private static Long getMillisForDay(String value) {
@@ -173,9 +169,9 @@ private static Long getMillisForDay(String value) {
     }
 
     private static long getTimeOffsetSeconds(String value) {
-        return (parseLong(value.substring(9, 11)) * 3600000L)
-                + (parseLong(value.substring(12, 14)) * 60000L)
-                + (parseLong(value.substring(15, LENGTH_INCL_SECONDS)) * 1000L);
+        return (IntConverter.parseLong(value.substring(9, 11)) * 3600000L)
+                + (IntConverter.parseLong(value.substring(12, 14)) * 60000L)
+                + (IntConverter.parseLong(value.substring(15, LENGTH_INCL_SECONDS)) * 1000L);
     }
 
     private static void verifyFormat(String value) throws FieldConvertError {

quickfixj-core/src/test/java/quickfix/FieldConvertersTest.java

@@ -53,6 +53,8 @@ public class FieldConvertersTest {
 
     @Test
     public void testIntegerConversion() throws Exception {
+        String intMaxValuePlus3 = "2147483650";
+        String intMinValueMinus3 = "-2147483651";
         assertEquals("123", IntConverter.convert(123));
         assertEquals(123, IntConverter.convert("123"));
         assertEquals(-1, IntConverter.convert("-1"));
@@ -75,6 +77,20 @@ public void testIntegerConversion() throws Exception {
         } catch (FieldConvertError e) {
             // expected
         }
+        // this should fail and not overflow
+        try {
+            IntConverter.convert(intMaxValuePlus3);
+            fail();
+        } catch (FieldConvertError e) {
+            // expected
+        }
+        // this should fail and not overflow
+        try {
+            IntConverter.convert(intMinValueMinus3);
+            fail();
+        } catch (FieldConvertError e) {
+            // expected
+        }
     }
 
     @Test