Fix CBOR decoding for seconds (travisbrown#300)

* Fix toString for time zones * Fix and partially test decoding for temporal literals with seconds
scala-steward · Jul 28, 2021 · 49ffe03 · 49ffe03
1 parent 19dc2e2
commit 49ffe03
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 4 deletions.
diff --git a/modules/core/src/main/java/org/dhallj/cbor/Reader.java b/modules/core/src/main/java/org/dhallj/cbor/Reader.java
@@ -1,5 +1,6 @@
 package org.dhallj.cbor;
 
+import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.nio.charset.Charset;
 import java.util.HashMap;
@@ -10,6 +11,7 @@
  * Dhall.
  */
 public abstract class Reader {
+  private static final BigInteger TWO = new BigInteger("2");
 
   /** Only allow symbols that correspond to entire encoded Dhall expressions. */
   public final <R> R nextSymbol(Visitor<R> visitor) {
@@ -87,6 +89,34 @@ public final BigInteger readBigNum() {
     }
   }
 
+  public final BigDecimal readBigDecimal() {
+    skip55799();
+    byte next = read();
+    switch (MajorType.fromByte(next)) {
+      case SEMANTIC_TAG:
+        AdditionalInfo info = AdditionalInfo.fromByte(next);
+        BigInteger t = readBigInteger(info, next);
+        long tag = t.longValue();
+        if (tag == 4) {
+          BigInteger length = readArrayStart();
+          if (length.equals(TWO)) {
+            BigInteger rawScale = readBigNum();
+            int scale = -rawScale.intValue();
+            BigInteger unscaledValue = readBigNum();
+
+            return new BigDecimal(unscaledValue, scale);
+          } else {
+            throw new CborException("Invalid decimal fraction");
+          }
+        } else {
+          throw new CborException(
+              Long.toString(tag) + " is not a valid tag for a decimal fraction");
+        }
+      default:
+        throw new CborException("Next symbol is not a decimal fraction");
+    }
+  }
+
   public final String readNullableTextString() {
     skip55799();
     byte next = read();

diff --git a/modules/core/src/main/java/org/dhallj/core/ToStringVisitor.java b/modules/core/src/main/java/org/dhallj/core/ToStringVisitor.java
@@ -111,9 +111,9 @@ public ToStringState onTime(Expr self, int hour, int minute, int second, BigDeci
 
   public ToStringState onTimeZone(Expr self, int minutes) {
     if (Long.signum(minutes) < 0) {
-      return new ToStringState("-" + pad2(-minutes / 60) + pad2(-minutes % 60));
+      return new ToStringState("-" + pad2(-minutes / 60) + ":" + pad2(-minutes % 60));
     } else {
-      return new ToStringState("+" + pad2(minutes / 60) + pad2(minutes % 60));
+      return new ToStringState("+" + pad2(minutes / 60) + ":" + pad2(minutes % 60));
     }
   }
 

diff --git a/modules/core/src/main/java/org/dhallj/core/binary/CborDecodingVisitor.java b/modules/core/src/main/java/org/dhallj/core/binary/CborDecodingVisitor.java
@@ -5,6 +5,7 @@
 import org.dhallj.core.Expr;
 import org.dhallj.core.Operator;
 
+import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -578,8 +579,11 @@ private Expr readTime(BigInteger length) {
     } else {
       BigInteger hour = this.reader.readUnsignedInteger();
       BigInteger minute = this.reader.readUnsignedInteger();
-      // TODO: read seconds.
-      return Expr.makeTimeLiteral(hour.intValue(), minute.intValue(), 0, java.math.BigDecimal.ZERO);
+      BigDecimal rawSeconds = this.reader.readBigDecimal();
+      int seconds = rawSeconds.intValue();
+      BigDecimal fractional = rawSeconds.subtract(new BigDecimal(seconds));
+
+      return Expr.makeTimeLiteral(hour.intValue(), minute.intValue(), seconds, fractional);
     }
   }
 

diff --git a/tests/src/test/scala/org/dhallj/tests/MiscSuite.scala b/tests/src/test/scala/org/dhallj/tests/MiscSuite.scala
@@ -1,7 +1,9 @@
 package org.dhallj.tests
 
+import java.time.Instant
 import munit.ScalaCheckSuite
 import org.dhallj.ast._
+import org.dhallj.core.binary.Decode
 import org.dhallj.core.Expr
 import org.dhallj.parser.DhallParser
 import org.scalacheck.{Arbitrary, Gen, Prop}
@@ -31,4 +33,29 @@ class MiscSuite extends ScalaCheckSuite {
   property("strings")(
     Prop.forAll((value: AsciiPrintableString) => parsesTo(s""""${value.value}"""", TextLiteral(value.value)))
   )
+
+  property("round-trip instants")(
+    Prop.forAll { (value: Instant) =>
+      val asString = value.toString
+      val parts = asString.split("-")
+
+      // We need a reasonable four-digit year.
+      val cleaned = if (parts(0).isEmpty) {
+        // If it's negative we just make something up.
+        "2021" + "-" + parts.drop(2).mkString("-")
+      } else if (parts(0).size != 4) {
+        // If it's not four digits we just make something up.
+        "1999" + "-" + parts.drop(1).mkString("-")
+      } else {
+        asString
+      }
+      val expected = Instant.parse(cleaned)
+      val result = Decode.decode(DhallParser.parse(cleaned).getEncodedBytes).toString
+
+      // Keep it low-tech.
+      val resultString = result.substring(8, 18) + "T" + result.substring(46).takeWhile(_ != '}') + "Z"
+
+      Instant.parse(resultString) == expected
+    }
+  )
 }