diff --git a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/EscrowFinish.java b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/EscrowFinish.java index 9aa5d8464..a1a7e7a5b 100644 --- a/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/EscrowFinish.java +++ b/xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/EscrowFinish.java @@ -228,9 +228,19 @@ default EscrowFinish normalizeCondition() { // In this case, we should try to read conditionRawValue to a Condition. If that fails, condition() // will remain empty, otherwise we will set condition(). try { - Condition condition = CryptoConditionReader.readCondition( - BaseEncoding.base16().decode(conditionRawValue().get().toUpperCase(Locale.US)) - ); + byte[] conditionRawValueBytes = BaseEncoding.base16() + .decode(conditionRawValue().get().toUpperCase(Locale.US)); + Condition condition = CryptoConditionReader.readCondition(conditionRawValueBytes); + + // CryptoConditionReader.readCondition can succeed if the first part of the condition raw value + // is a valid condition, but the raw value has extra bytes afterward. Here, we check that the parsed + // condition is equivalent to the raw value when re-written. + if (!Arrays.equals(CryptoConditionWriter.writeCondition(condition), conditionRawValueBytes)) { + logger.warn("EscrowFinish Condition was malformed: mismatch between raw value and parsed condition. " + + "conditionRawValue() will contain the condition value, but condition() will be empty."); + return this; + } + return EscrowFinish.builder().from(this) .condition(condition) .build(); @@ -300,9 +310,19 @@ default EscrowFinish normalizeFulfillment() { // In this case, we should try to read fulfillmentRawValue to a Condition. If that fails, fulfillment() // will remain empty, otherwise we will set fulfillment(). try { - Fulfillment fulfillment = CryptoConditionReader.readFulfillment( - BaseEncoding.base16().decode(fulfillmentRawValue().get().toUpperCase(Locale.US)) - ); + byte[] fulfillmentRawValueBytes = BaseEncoding.base16() + .decode(fulfillmentRawValue().get().toUpperCase(Locale.US)); + Fulfillment fulfillment = CryptoConditionReader.readFulfillment(fulfillmentRawValueBytes); + + // CryptoConditionReader.readFulfillment can succeed if the first part of the fulfillment raw value + // is a valid fulfillment, but the raw value has extra bytes afterward. Here, we check that the parsed + // fulfillment is equivalent to the raw value when re-written. + if (!Arrays.equals(CryptoConditionWriter.writeFulfillment(fulfillment), fulfillmentRawValueBytes)) { + logger.warn("EscrowFinish Fulfillment was malformed: mismatch between raw value and parsed fulfillment. " + + "fulfillmentRawValue() will contain the fulfillment value, but fulfillment() will be empty."); + return this; + } + return EscrowFinish.builder().from(this) .fulfillment(fulfillment) .build(); @@ -318,7 +338,7 @@ default EscrowFinish normalizeFulfillment() { } } catch (DerEncodingException e) { - // This should never happen. CryptoconditionWriter.writeCondition errantly declares that it can throw + // This should never happen. CryptoconditionWriter.writeFulfillment errantly declares that it can throw // a DerEncodingException, but nowhere in its implementation does it throw. throw new RuntimeException(e); } diff --git a/xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/EscrowFinishTest.java b/xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/EscrowFinishTest.java index 232bf6b6b..15358b4c6 100644 --- a/xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/EscrowFinishTest.java +++ b/xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/EscrowFinishTest.java @@ -28,6 +28,7 @@ import com.google.common.primitives.UnsignedInteger; import com.ripple.cryptoconditions.Condition; import com.ripple.cryptoconditions.CryptoConditionReader; +import com.ripple.cryptoconditions.CryptoConditionWriter; import com.ripple.cryptoconditions.Fulfillment; import com.ripple.cryptoconditions.PreimageSha256Condition; import com.ripple.cryptoconditions.PreimageSha256Fulfillment; @@ -176,6 +177,29 @@ void normalizeWithNoConditionAndRawValueForMalformedCondition() { assertThat(actual.conditionRawValue()).isNotEmpty().get().isEqualTo("1234"); } + /** + * This tests the case where conditionRawValue is present and is parseable to a Condition but when the + * parsed Condition is written to a byte array, the value differs from the conditionRawValue bytes. This + * can occur if the condition raw value contains a valid condition in the first 32 bytes, but also includes + * extra bytes afterward. + */ + @Test + void normalizeConditionWithRawValueThatIsParseableButNotValid() { + EscrowFinish actual = EscrowFinish.builder() + .fee(XrpCurrencyAmount.ofDrops(1)) + .account(Address.of("rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59Ba")) + .sequence(UnsignedInteger.ONE) + .owner(Address.of("rN7n7otQDd6FczFgLdSqtcsAUxDkw6fzRH")) + .offerSequence(UnsignedInteger.ZERO) + .conditionRawValue(GOOD_CONDITION_STR + GOOD_CONDITION_STR) + .build(); + + assertThat(actual.condition()).isEmpty(); + assertThat(actual.conditionRawValue()).isNotEmpty().get().isEqualTo( + GOOD_CONDITION_STR + GOOD_CONDITION_STR + ); + } + @Test void normalizeWithNoConditionAndRawValueForBadHexLengthCondition() { EscrowFinish actual = EscrowFinish.builder() @@ -284,6 +308,29 @@ void normalizeWithNoFulfillmentAndRawValueForMalformedFulfillment() { assertThat(actual.fulfillmentRawValue()).isNotEmpty().get().isEqualTo("1234"); } + /** + * This tests the case where fulfillmentRawValue is present and is parseable to a Fulfillment but when the + * parsed Fulfillment is written to a byte array, the value differs from the fulfillmentRawValue bytes. This + * can occur if the fulfillment raw value contains a valid fulfillment in the first 32 bytes, but also includes + * extra bytes afterward, such as in transaction 138543329687544CDAFCD3AB0DCBFE9C4F8E710397747BA7155F19426F493C8D. + */ + @Test + void normalizeFulfillmentWithRawValueThatIsParseableButNotValid() { + EscrowFinish actual = EscrowFinish.builder() + .fee(XrpCurrencyAmount.ofDrops(1)) + .account(Address.of("rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59Ba")) + .sequence(UnsignedInteger.ONE) + .owner(Address.of("rN7n7otQDd6FczFgLdSqtcsAUxDkw6fzRH")) + .offerSequence(UnsignedInteger.ZERO) + .fulfillmentRawValue(GOOD_FULFILLMENT_STR + GOOD_FULFILLMENT_STR) + .build(); + + assertThat(actual.fulfillment()).isEmpty(); + assertThat(actual.fulfillmentRawValue()).isNotEmpty().get().isEqualTo( + GOOD_FULFILLMENT_STR + GOOD_FULFILLMENT_STR + ); + } + @Test void normalizeWithNoFulfillmentAndRawValueForBadHexLengthFulfillment() { EscrowFinish actual = EscrowFinish.builder()