diff --git a/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java b/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java index e94ef24d2..7af5ed21d 100644 --- a/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/FodId.java +++ b/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. *

- * 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 diff --git a/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/RedeemResult.java b/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/RedeemResult.java index a2625f942..fbeeca1b0 100644 --- a/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/RedeemResult.java +++ b/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; } } diff --git a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientTests.java b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientTests.java index 501a2cabf..08f8af717 100644 --- a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientTests.java +++ b/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() { diff --git a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java index fec3bafaa..360f9ca70 100644 --- a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/FodIdParseTests.java +++ b/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