Compare 51Did factor words ignoring case, and name the version of a short payload

pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java

@@ -257,29 +257,35 @@ private static FodIdParseResult read(OwidParseResult envelope) {
      * already read or signed. This is the one walk of the payload that every
      * reader, throwing or not, goes through.
      * <p>
-     * The rules are lower bounds only. The header must be present before the
-     * type can be read, and the type then sets the least the payload can
-     * hold. Anything longer is accepted as it stands, because the bytes past
-     * the match key are the Terms and then a creator context section whose
-     * shape the cloud judges. The Terms adds nothing to those bounds, since
-     * a payload that ends at the match key reads as a Terms of zero.
+     * The version is read from the first byte before anything else, so a
+     * payload of a version this package does not know is refused naming
+     * that version however short it is. The other rules are lower bounds
+     * only. The header must be present before the type can be read, and the
+     * type then sets the least the payload can hold. Anything longer is
+     * accepted as it stands, because the bytes past the match key are the
+     * Terms and then a creator context section whose shape the cloud
+     * judges. The Terms adds nothing to those bounds, since a payload that
+     * ends at the match key reads as a Terms of zero.
      */
     private static FodIdParseResult read(Owid owid) {
         byte[] payload = owid.getPayload();
-        if (payload.length < HEADER_LENGTH) {
+        if (payload.length <= FLAGS_OFFSET) {
             return FodIdParseResult.failed(FodIdParseStatus.PAYLOAD_TOO_SHORT);
         }
         int flags = payload[FLAGS_OFFSET] & 0xFF;
-        // The version is read before any field, because a later version
-        // exists precisely because a field moved. Reading a payload of a
-        // version this package does not know under the layout it does know
-        // would answer with values that are wrong rather than absent,
-        // which is worse than refusing, and a version that nothing checks
-        // protects nothing.
+        // The version is read before any field, and before the header's
+        // length is checked, because a later version exists precisely
+        // because a field moved. Reading a payload of a version this package
+        // does not know under the layout it does know would answer with
+        // values that are wrong rather than absent, which is worse than
+        // refusing, and a version that nothing checks protects nothing.
         int payloadVersion = payloadVersionOf(flags);
         if (payloadVersion != SUPPORTED_PAYLOAD_VERSION) {
             return FodIdParseResult.unsupportedPayloadVersion(payloadVersion);
         }
+        if (payload.length < HEADER_LENGTH) {
+            return FodIdParseResult.failed(FodIdParseStatus.PAYLOAD_TOO_SHORT);
+        }
         // A payload with no usage bit set states no usage at all. The cloud
         // never writes one, so it is damaged or forged, and it is refused
         // here rather than read, because the only safe answer to it is not

pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/RedeemResult.java

@@ -28,6 +28,7 @@
 import java.time.format.DateTimeParseException;
 import java.util.Collections;
 import java.util.LinkedHashMap;
+import java.util.Locale;
 import java.util.Map;
 
 /**
@@ -196,15 +197,17 @@ public String getValue() {
          * Verified, misconfigured and notrecorded are each read on their own,
          * because none of them is a mismatch. Anything else, including a word
          * this client does not know, is a mismatch, so an unexpected value
-         * never reads as a pass.
+         * never reads as a pass. The comparison ignores case, so the word is
+         * lower-cased in the root locale first.
          *
          * @param value the factor's string from the cloud, or null
          * @return the matching outcome, or {@link #MISMATCH}
          */
         public static Factor fromValue(String value) {
             if (value != null) {
+                String word = value.toLowerCase(Locale.ROOT);
                 for (Factor factor : values()) {
-                    if (factor.value.equals(value)) {
+                    if (factor.value.equals(word)) {
                         return factor;
                     }
                 }

pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientTests.java

@@ -42,6 +42,7 @@
 import java.util.Collections;
 import java.util.Deque;
 import java.util.List;
+import java.util.Locale;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.Executor;
@@ -1038,6 +1039,45 @@ public void redeem_UnknownFactorValue_IsStillAMismatch()
             result.getFactors().get("transport"));
     }
 
+    /**
+     * The comparison of a factor word ignores case, so each word in other
+     * letters reads as its own outcome rather than as a mismatch. The
+     * default locale is Turkish, where lower-casing an I in the default
+     * locale gives a dotless i.
+     */
+    @Test
+    public void redeem_FactorWordsInOtherLetters_ReadAsTheirOutcome()
+            throws Exception {
+        Locale saved = Locale.getDefault();
+        Locale.setDefault(Locale.forLanguageTag("tr-TR"));
+        try {
+            transport.queue(200, "{\"signature\":\"verified\","
+                + "\"context\":\"mismatch\","
+                + "\"factors\":{\"transport\":\"Verified\","
+                + "\"device\":\"NotRecorded\","
+                + "\"browserip\":\"MISCONFIGURED\","
+                + "\"connectionip\":\"VERIFIED\","
+                + "\"asn\":\"Mismatch\"}}");
+            FodId fodId = key2.fodIdAt(canonicalPayload(), WEEK2);
+
+            RedeemResult result =
+                client.redeem(fodId, "sealed", "abc").join();
+
+            assertEquals(RedeemResult.Factor.VERIFIED,
+                result.getFactors().get("transport"));
+            assertEquals(RedeemResult.Factor.NOT_RECORDED,
+                result.getFactors().get("device"));
+            assertEquals(RedeemResult.Factor.MISCONFIGURED,
+                result.getFactors().get("browserip"));
+            assertEquals(RedeemResult.Factor.VERIFIED,
+                result.getFactors().get("connectionip"));
+            assertEquals(RedeemResult.Factor.MISMATCH,
+                result.getFactors().get("asn"));
+        } finally {
+            Locale.setDefault(saved);
+        }
+    }
+
     /** Every outcome reports the cloud's own word for itself. */
     @Test
     public void factor_GetValue_IsTheCloudsWord() {

pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java

@@ -491,6 +491,47 @@ public void version_NotZero_IsRefused() throws Exception {
         }
     }
 
+    /**
+     * The version is read from the first byte, before the layout that sets
+     * the header's length, so a payload too short for that header is still
+     * refused naming the version it gives. A short payload of version 0 is
+     * too short.
+     */
+    @Test
+    public void version_ShortPayload_IsRefusedNamingTheVersion()
+            throws Exception {
+        for (int length = 1; length < FodId.HEADER_LENGTH; length++) {
+            byte[] shortPayload = Arrays.copyOf(canonicalPayload(), length);
+
+            assertEquals("length " + length + " version 0",
+                FodIdParseStatus.PAYLOAD_TOO_SHORT,
+                FodId.tryFromBase64(factory
+                    .signedOwidAt(withVersion(shortPayload, 0), DATE)
+                    .asBase64()).getStatus());
+
+            for (int version : new int[] { 1, 2, 3 }) {
+                String base64 = factory
+                    .signedOwidAt(withVersion(shortPayload, version), DATE)
+                    .asBase64();
+                String label = "length " + length + " version " + version;
+
+                FodIdParseResult result = FodId.tryFromBase64(base64);
+                assertEquals(label,
+                    FodIdParseStatus.UNSUPPORTED_PAYLOAD_VERSION,
+                    result.getStatus());
+                assertEquals(label, version, result.getPayloadVersion());
+                assertNull(label, result.getValue());
+                try {
+                    FodId.fromBase64(base64);
+                    fail(label + " should have been refused");
+                } catch (IllegalArgumentException thrown) {
+                    assertTrue(label + ": " + thrown.getMessage(),
+                        thrown.getMessage().contains("version " + version));
+                }
+            }
+        }
+    }
+
     /**
      * The throwing readers name the version they found, so whoever reads
      * the message knows which layout the identifier claims rather than only