From 775d1bc02f3f717bf6066ecf8987ecaea0c9524c Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Tue, 17 Nov 2015 22:32:48 +0000
Subject: [PATCH] OPENDJ-2397: fix sort order of generalized time values older than 1970

---
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeGreaterThanMatchingRuleTest.java |    2 +
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java                                |   20 ++++++---
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleTest.java |    4 ++
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeLessThanMatchingRuleTest.java    |    2 +
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleImpl.java |   11 +----
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeEqualityMatchingRuleImpl.java |   18 +++++++-
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/TimeBasedMatchingRulesImpl.java              |   12 ++----
 7 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java
index 006d69b..cdb5167 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/GeneralizedTime.java
@@ -21,7 +21,7 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2012-2013 ForgeRock AS.
+ *      Copyright 2012-2015 ForgeRock AS.
  */
 package org.forgerock.opendj.ldap;
 
@@ -58,6 +58,12 @@
     /** UTC TimeZone is assumed to never change over JVM lifetime. */
     private static final TimeZone TIME_ZONE_UTC_OBJ = TimeZone.getTimeZone("UTC");
 
+    /** The smallest time representable using the generalized time syntax. */
+    public static final GeneralizedTime MIN_GENERALIZED_TIME = valueOf("00010101000000Z");
+
+    /** The smallest time in milli-seconds representable using the generalized time syntax. */
+    public static final long MIN_GENERALIZED_TIME_MS = MIN_GENERALIZED_TIME.getTimeInMillis();
+
     /**
      * Returns a generalized time whose value is the current time, using the
      * default time zone and locale.
@@ -80,7 +86,7 @@
      */
     public static GeneralizedTime valueOf(final Calendar calendar) {
         Reject.ifNull(calendar);
-        return new GeneralizedTime((Calendar) calendar.clone(), null, -1L, null);
+        return new GeneralizedTime((Calendar) calendar.clone(), null, Long.MIN_VALUE, null);
     }
 
     /**
@@ -95,7 +101,7 @@
      */
     public static GeneralizedTime valueOf(final Date date) {
         Reject.ifNull(date);
-        return new GeneralizedTime(null, (Date) date.clone(), -1L, null);
+        return new GeneralizedTime(null, (Date) date.clone(), Long.MIN_VALUE, null);
     }
 
     /**
@@ -108,7 +114,7 @@
      *         since the epoch.
      */
     public static GeneralizedTime valueOf(final long timeMS) {
-        Reject.ifFalse(timeMS >= 0, "timeMS must be >= 0");
+        Reject.ifTrue(timeMS < MIN_GENERALIZED_TIME_MS, "timeMS is too old to represent as a generalized time");
         return new GeneralizedTime(null, null, timeMS, null);
     }
 
@@ -548,7 +554,7 @@
             calendar.setTimeZone(tz);
             calendar.set(year, month, day, hour, minute, second);
             calendar.set(Calendar.MILLISECOND, 0);
-            return new GeneralizedTime(calendar, null, -1L, value);
+            return new GeneralizedTime(calendar, null, Long.MIN_VALUE, value);
         } catch (final Exception e) {
             // This should only happen if the provided date wasn't legal
             // (e.g., September 31).
@@ -665,7 +671,7 @@
             calendar.setTimeZone(timeZone);
             calendar.set(year, month, day, hour, minute, second);
             calendar.set(Calendar.MILLISECOND, additionalMilliseconds);
-            return new GeneralizedTime(calendar, null, -1L, value);
+            return new GeneralizedTime(calendar, null, Long.MIN_VALUE, value);
         } catch (final Exception e) {
             // This should only happen if the provided date wasn't legal
             // (e.g., September 31).
@@ -841,7 +847,7 @@
      */
     public long getTimeInMillis() {
         long tmpTimeMS = timeMS;
-        if (tmpTimeMS == -1) {
+        if (tmpTimeMS == Long.MIN_VALUE) {
             if (date != null) {
                 tmpTimeMS = date.getTime();
             } else {
diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeEqualityMatchingRuleImpl.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeEqualityMatchingRuleImpl.java
index 3f0ce78..d7dce93 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeEqualityMatchingRuleImpl.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeEqualityMatchingRuleImpl.java
@@ -44,12 +44,24 @@
         super(EMR_GENERALIZED_TIME_NAME);
     }
 
-    public ByteString normalizeAttributeValue(final Schema schema, final ByteSequence value)
-            throws DecodeException {
+    public ByteString normalizeAttributeValue(final Schema schema, final ByteSequence value) throws DecodeException {
+        return normalizeAttributeValue(value);
+    }
+
+    static ByteString normalizeAttributeValue(final ByteSequence value) throws DecodeException {
         try {
-            return ByteString.valueOfLong(GeneralizedTime.valueOf(value.toString()).getTimeInMillis());
+            final GeneralizedTime time = GeneralizedTime.valueOf(value.toString());
+            return createNormalizedAttributeValue(time.getTimeInMillis());
         } catch (LocalizedIllegalArgumentException e) {
             throw DecodeException.error(e.getMessageObject());
         }
     }
+
+    static ByteString createNormalizedAttributeValue(final long timeInMillis) {
+        /* Dates older than 1970 will be negative and will sort after dates more recent than 1970 due to twos
+         * complement encoding. Therefore mangle the time in order to ensure that it is positive for all valid values
+         * of a generalized time.
+         */
+        return ByteString.valueOfLong(timeInMillis - GeneralizedTime.MIN_GENERALIZED_TIME_MS);
+    }
 }
diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleImpl.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleImpl.java
index 36d531f..3107ae0 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleImpl.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleImpl.java
@@ -28,11 +28,9 @@
 
 import static org.forgerock.opendj.ldap.schema.SchemaConstants.*;
 
-import org.forgerock.i18n.LocalizedIllegalArgumentException;
 import org.forgerock.opendj.ldap.ByteSequence;
 import org.forgerock.opendj.ldap.ByteString;
 import org.forgerock.opendj.ldap.DecodeException;
-import org.forgerock.opendj.ldap.GeneralizedTime;
 
 /**
  * This class defines the generalizedTimeOrderingMatch matching rule defined in
@@ -45,12 +43,7 @@
         super(EMR_GENERALIZED_TIME_NAME);
     }
 
-    public ByteString normalizeAttributeValue(final Schema schema, final ByteSequence value)
-            throws DecodeException {
-        try {
-            return ByteString.valueOfLong(GeneralizedTime.valueOf(value.toString()).getTimeInMillis());
-        } catch (LocalizedIllegalArgumentException e) {
-            throw DecodeException.error(e.getMessageObject());
-        }
+    public ByteString normalizeAttributeValue(final Schema schema, final ByteSequence value) throws DecodeException {
+        return GeneralizedTimeEqualityMatchingRuleImpl.normalizeAttributeValue(value);
     }
 }
diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/TimeBasedMatchingRulesImpl.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/TimeBasedMatchingRulesImpl.java
index dc46c6a..c40fc90 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/TimeBasedMatchingRulesImpl.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/TimeBasedMatchingRulesImpl.java
@@ -34,7 +34,6 @@
 import java.util.List;
 import java.util.TimeZone;
 
-import org.forgerock.i18n.LocalizedIllegalArgumentException;
 import org.forgerock.opendj.ldap.Assertion;
 import org.forgerock.opendj.ldap.ByteSequence;
 import org.forgerock.opendj.ldap.ByteSequenceReader;
@@ -51,6 +50,7 @@
 import static com.forgerock.opendj.ldap.CoreMessages.*;
 import static com.forgerock.opendj.util.StaticUtils.*;
 import static org.forgerock.opendj.ldap.DecodeException.*;
+import static org.forgerock.opendj.ldap.schema.GeneralizedTimeEqualityMatchingRuleImpl.createNormalizedAttributeValue;
 import static org.forgerock.opendj.ldap.schema.SchemaConstants.*;
 
 /** Implementations of time-based matching rules. */
@@ -105,11 +105,7 @@
 
         @Override
         public final ByteString normalizeAttributeValue(Schema schema, ByteSequence value) throws DecodeException {
-            try {
-                return ByteString.valueOfLong(GeneralizedTime.valueOf(value.toString()).getTimeInMillis());
-            } catch (final LocalizedIllegalArgumentException e) {
-                throw error(e.getMessageObject());
-            }
+            return GeneralizedTimeEqualityMatchingRuleImpl.normalizeAttributeValue(value);
         }
 
         /** Utility method to convert the provided integer and the provided byte representing a digit to an integer. */
@@ -216,7 +212,7 @@
 
             long delta = (second + minute * 60 + hour * 3600 + day * 24 * 3600 + week * 7 * 24 * 3600) * 1000;
             long now = timeService.now();
-            return ByteString.valueOfLong(signed ? now - delta : now + delta);
+            return createNormalizedAttributeValue(signed ? now - delta : now + delta);
         }
 
     }
@@ -491,7 +487,7 @@
             // Build the information from the attribute value.
             GregorianCalendar cal = new GregorianCalendar(TIME_ZONE_UTC);
             cal.setLenient(false);
-            cal.setTimeInMillis(((ByteString) attributeValue).toLong());
+            cal.setTimeInMillis(attributeValue.toByteString().toLong() + GeneralizedTime.MIN_GENERALIZED_TIME_MS);
             int second = cal.get(Calendar.SECOND);
             int minute = cal.get(Calendar.MINUTE);
             int hour = cal.get(Calendar.HOUR_OF_DAY);
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleTest.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleTest.java
index 7637195..ca7ab82 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleTest.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/GeneralizedTimeOrderingMatchingRuleTest.java
@@ -22,6 +22,7 @@
  *
  *
  *      Copyright 2009 Sun Microsystems, Inc.
+ *      Portions Copyright 2015 ForgeRock AS.
  */
 package org.forgerock.opendj.ldap.schema;
 
@@ -73,6 +74,9 @@
             {"20060912180129.000Z", "20060912180130.001Z", -1},
             {"20060912180129.1Z",   "20060912180130.2Z",   -1},
             {"20060912180129.11Z",  "20060912180130.12Z",  -1},
+            // OPENDJ-2397 - dates before 1970 have negative ms.
+            {"19000101010203Z",     "20000101010203Z",     -1},
+            {"20000101010203Z",     "19000101010203Z",      1},
         };
     }
 
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeGreaterThanMatchingRuleTest.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeGreaterThanMatchingRuleTest.java
index a8312e7..ff05a00 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeGreaterThanMatchingRuleTest.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeGreaterThanMatchingRuleTest.java
@@ -107,6 +107,8 @@
             { nowGT, "+1h", ConditionResult.FALSE },
             { nowGT, "+1m", ConditionResult.FALSE },
             { nowGT, "+1w", ConditionResult.FALSE },
+            // OPENDJ-2397 - dates before 1970 have negative ms.
+            {"19000101010203Z", "1d", ConditionResult.FALSE},
         };
     }
 
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeLessThanMatchingRuleTest.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeLessThanMatchingRuleTest.java
index 743b8d9..5a02902 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeLessThanMatchingRuleTest.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/RelativeTimeLessThanMatchingRuleTest.java
@@ -108,6 +108,8 @@
             { nowGT, "-2w", ConditionResult.FALSE },
             { nowGT, "-10m", ConditionResult.FALSE },
             { nowGT, "-1s", ConditionResult.FALSE },
+            // OPENDJ-2397 - dates before 1970 have negative ms.
+            {"19000101010203Z", "1d", ConditionResult.TRUE},
         };
     }
 

--
Gitblit v1.10.0