Wire-contract parity: serializer, date split, payment_details discriminator, and error mapping

riskified-sdk/src/main/java/com/riskified/JSONFormater.java

@@ -2,10 +2,11 @@
 
 import java.lang.reflect.Type;
 import java.text.DateFormat;
-import java.text.SimpleDateFormat;
 import java.util.Date;
 
 import com.google.gson.*;
+import com.riskified.adapters.NaiveDateTypeAdapter;
+import com.riskified.adapters.PaymentDetailsAdapterFactory;
 import com.riskified.models.BankWirePaymentDetails;
 import com.riskified.models.CreditCardPaymentDetails;
 import com.riskified.models.IPaymentDetails;
@@ -18,17 +19,60 @@ public class JSONFormater {
 	public static String toJson(Object obj) {
         Gson gson = new GsonBuilder().setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
                 .registerTypeAdapter(Date.class, new DateTimeSerializer())
-                .registerTypeAdapterFactory(paymentDetailsSerializer())
+                .registerTypeAdapterFactory(new PaymentDetailsAdapterFactory())
                 .create();
         return gson.toJson(obj);
     }
 
+    /**
+     * Serializer for the 25 <em>offset-bearing</em> date fields of the wire contract — every
+     * {@link Date} that is not explicitly annotated with
+     * {@code @JsonAdapter(NaiveDateTypeAdapter.class)}.
+     *
+     * <p>
+     * Two properties matter here:
+     * <ul>
+     * <li><b>UTC, not the JVM default timezone.</b> The previous implementation used the default
+     * timezone, so the same {@link Date} serialized differently on two machines — and since the
+     * HMAC is computed over the serialized bytes, so did the signature.</li>
+     * <li><b>The offset is written as {@code +00:00}, not {@code Z}.</b> Both are valid ISO 8601 and
+     * denote the same instant, but the reference implementation renders a zero-offset
+     * {@code DateTimeOffset} as {@code +00:00}, and byte parity with the reference is the point.
+     * The literal is safe because the formatter is pinned to UTC one line below.</li>
+     * </ul>
+     *
+     * @see NaiveDateTypeAdapter for the 13 fields that must carry no offset at all
+     */
     public static class DateTimeSerializer implements JsonSerializer<Date> {
+        /** Zero offset spelled out, to match how .NET renders a UTC {@code DateTimeOffset}. */
+        public static final String OFFSET_PATTERN = "yyyy-MM-dd'T'HH:mm:ss'+00:00'";
+
         public JsonElement serialize(Date src, Type typeOfSrc, JsonSerializationContext context) {
-            DateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssXXX");
+            DateFormat df = NaiveDateTypeAdapter.utcFormat(OFFSET_PATTERN);
             return new JsonPrimitive(df.format(src));
         }
     }
+
+    /**
+     * The polymorphic {@code payment_details} adapter that this SDK used to register.
+     *
+     * <p>
+     * <b>No longer used, and deliberately so.</b> It injected a {@code "method"} key into every
+     * {@code payment_details} object with values ({@code credit_card}, {@code bank_wire},
+     * {@code digital_wallet}) that disagreed with the {@code payment_type} value emitted alongside
+     * it. The wire contract carries <b>no</b> type discriminator on {@code payment_details}: the
+     * variant is expressed by which keys are present, with the constant {@code payment_type} acting
+     * as the de-facto discriminator. No other SDK in the fleet emitted {@code method}.
+     *
+     * <p>
+     * Gson dispatches on the runtime type of each element of a {@code List<IPaymentDetails>} on its
+     * own, so nothing is lost by dropping the factory. It is retained here only so that callers who
+     * built their own {@code Gson} against it still compile; do not register it.
+     *
+     * @return the legacy discriminator-injecting factory
+     * @deprecated the {@code method} key it emits is not part of the Riskified wire contract.
+     */
+    @Deprecated
     public static RuntimeTypeAdapterFactory paymentDetailsSerializer() {
         return RuntimeTypeAdapterFactory
                 .of(IPaymentDetails.class, "method")
@@ -38,5 +82,5 @@ public static RuntimeTypeAdapterFactory paymentDetailsSerializer() {
                 .registerSubtype(BankWirePaymentDetails.class, "bank_wire")
                 .registerSubtype(WalletPaymentDetails.class, "digital_wallet");
     }
-   
+
 }

riskified-sdk/src/main/java/com/riskified/RiskifiedClient.java

@@ -5,6 +5,8 @@
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonSyntaxException;
 import com.riskified.models.*;
+// Explicit: com.riskified.models.Error would otherwise be ambiguous with java.lang.Error.
+import com.riskified.models.Error;
 import com.riskified.validations.FieldBadFormatException;
 import com.riskified.validations.IValidated;
 import com.riskified.validations.Validation;
@@ -901,22 +903,60 @@ private Response postCheckoutOrder(Object data, String url) throws IOException,
         response = executeClient(client, request);
         String postBody = EntityUtils.toString(response.getEntity(), "UTF-8");
         int status = response.getStatusLine().getStatusCode();
-        Response responseObject = getCheckoutResponseObject(postBody);
-        switch (status) {
-            case 200:
-                return responseObject;
-            case 400:
-                throw new HttpResponseException(status, responseObject.getError().getMessage());
-            case 401:
-                throw new HttpResponseException(status, responseObject.getError().getMessage());
-            case 404:
-                throw new HttpResponseException(status, responseObject.getError().getMessage());
-            case 429:
-                throw new HttpResponseException(status, responseObject.getError().getMessage());			
-            case 504:
-                throw new HttpResponseException(status, "Temporary error, please retry");
-            default:
-                throw new HttpResponseException(500, "Contact Riskified support");
+        String statusText = response.getStatusLine().getReasonPhrase();
+        if (status == 200) {
+            return getCheckoutResponseObject(postBody);
+        }
+        // Parsing happens only after the 200 check, and never decides whether the call failed.
+        // Previously the body was parsed first and dereferenced unguarded, so any error body that
+        // was not the {"error":{"message":...}} shape raised a NullPointerException in place of the
+        // HTTP error the caller was waiting for.
+        throw buildHttpException(status, statusText, postBody, tryParseCheckoutResponse(postBody));
+    }
+
+    /**
+     * Turns a non-2xx response into an exception that keeps the real status, the raw body and the
+     * parsed error, whatever shape the body turned out to be.
+     *
+     * <p>
+     * There is deliberately no {@code default} case rewriting the status: an unmatched status is
+     * reported as itself. Collapsing 502, 503 and 500 into one substituted 500 with the body
+     * discarded is what used to make retry decisions impossible.
+     */
+    static RiskifiedHttpException buildHttpException(int status, String statusText, String body, Response parsed) {
+        Error error = parsed == null ? null : parsed.getError();
+        String message;
+        if (status == 504) {
+            // Documented by the CBG and Account Secure specs with no content schema.
+            message = "Temporary error, please retry";
+        } else if (error != null && error.getMessage() != null && !error.getMessage().isEmpty()) {
+            message = error.getMessage();
+        } else if (body != null && !body.trim().isEmpty()) {
+            message = body;
+        } else if (statusText != null && !statusText.isEmpty()) {
+            message = statusText;
+        } else {
+            message = "HTTP " + status;
+        }
+        return new RiskifiedHttpException(status, statusText, body, error, message);
+    }
+
+    /**
+     * Best-effort parse of an error body into the common {@code {"error":{"message":...}}} shape.
+     * Returns {@code null} for any of the other six documented shapes, for a bare JSON string, and
+     * for an empty or malformed body. Never throws: the caller already knows the request failed and
+     * the raw body is preserved regardless.
+     */
+    private static Response tryParseCheckoutResponse(String postBody) {
+        try {
+            CheckoutResponse res = new Gson().fromJson(postBody, CheckoutResponse.class);
+            if (res == null) {
+                return null;
+            }
+            res.setOrder(res.getCheckout());
+            return res;
+        } catch (RuntimeException e) {
+            return null;
         }
     }
 
@@ -1014,26 +1054,20 @@ private Response postOrder(Object data, String url) throws IOException {
         HttpResponse response;
         HttpClient client = constructHttpClient();
         response = executeClient(client, request);
-        String postBody = EntityUtils.toString(response.getEntity());
+        // Explicit UTF-8. Without it EntityUtils falls back to ISO-8859-1 whenever the response
+        // omits a charset parameter, which turns any non-ASCII error message into mojibake --
+        // and this body is now preserved verbatim on the exception, so mangling it here would
+        // corrupt the thing the caller is meant to read. postCheckoutOrder already did this.
+        String postBody = EntityUtils.toString(response.getEntity(), "UTF-8");
         int status = response.getStatusLine().getStatusCode();
 
-        Response responseObject = getResponseObject(postBody);
-        switch (status) {
-	        case 200:
-	            return responseObject;
-	        case 400:
-	            throw new HttpResponseException(status, postBody);
-	        case 401:
-	            throw new HttpResponseException(status, postBody);
-	        case 404:
-	            throw new HttpResponseException(status, postBody);
-		case 429: 
-		    throw new HttpResponseException(status, postBody);
-	        case 504:
-	            throw new HttpResponseException(status, "Temporary error, please retry");
-	        default:
-	            throw new HttpResponseException(500, "Contact Riskified support");
-	    }
+        String statusText = response.getStatusLine().getReasonPhrase();
+
+        if (status == 200) {
+            return getResponseObject(postBody);
+        }
+        // Same rule as the checkout path: report the status the server sent, keep the body.
+        throw buildHttpException(status, statusText, postBody, getResponseObject(postBody));
     }
 
     private Response getResponseObject(String postBody) throws IOException {

riskified-sdk/src/main/java/com/riskified/RiskifiedHttpException.java

@@ -0,0 +1,78 @@
+package com.riskified;
+
+import com.riskified.models.Error;
+import org.apache.http.client.HttpResponseException;
+
+/**
+ * A non-2xx response from the Riskified API, with the transport facts preserved as structured
+ * fields rather than flattened into a message string.
+ *
+ * <p>
+ * It extends {@link HttpResponseException}, so every existing {@code catch (HttpResponseException)}
+ * keeps working and {@link #getStatusCode()} keeps returning what it always did. What is new is
+ * that the real status now survives: the client used to rewrite <b>every</b> unmatched status to
+ * {@code 500 "Contact Riskified support"} and discard the body, which made a 502, a 503 and a
+ * genuine 500 indistinguishable to a caller trying to decide whether to retry.
+ *
+ * <p>
+ * {@link #getResponseBody()} carries the raw body as a separate field so that logging can redact
+ * it. Response bodies can contain customer PII, and a message string is the one place logging
+ * cannot reach — a defect the reference implementation has
+ * ({@code Riskified.SDK/Utils/HttpUtils.cs:224}) and that this SDK should not deepen.
+ *
+ * <p>
+ * The API returns seven mutually incompatible error body shapes across its six specs
+ * ({@code docs/flows/00-shared-contract.md} section 6). {@link #getError()} is populated only when
+ * the body matched the common {@code {"error": {"message": ...}}} shape; for the other six it is
+ * {@code null} and the body is still available verbatim. It is never a reason to fail.
+ *
+ * @since 6.4.1
+ */
+public class RiskifiedHttpException extends HttpResponseException {
+
+    private static final long serialVersionUID = 1L;
+
+    private final String statusText;
+    private final String responseBody;
+    private final Error error;
+
+    /**
+     * @param statusCode   the real HTTP status code, never a substituted one
+     * @param statusText   the HTTP reason phrase, may be {@code null}
+     * @param responseBody the raw response body, may be {@code null}
+     * @param error        the parsed error object when the body matched the common shape, else
+     *                     {@code null}
+     * @param message      the exception message
+     */
+    public RiskifiedHttpException(int statusCode, String statusText, String responseBody, Error error,
+            String message) {
+        super(statusCode, message);
+        this.statusText = statusText;
+        this.responseBody = responseBody;
+        this.error = error;
+    }
+
+    /**
+     * @return the HTTP reason phrase, or {@code null} if the response carried none
+     */
+    public String getStatusText() {
+        return statusText;
+    }
+
+    /**
+     * @return the raw response body, or {@code null} if the response had none. Attach this to a
+     *         structured log field, not to a message string — it can contain customer PII.
+     */
+    public String getResponseBody() {
+        return responseBody;
+    }
+
+    /**
+     * @return the parsed error, or {@code null} when the body did not match the common
+     *         {@code {"error": {"message": ...}}} shape. A {@code null} here says nothing about
+     *         whether the request failed.
+     */
+    public Error getError() {
+        return error;
+    }
+}

riskified-sdk/src/main/java/com/riskified/SHA256Handler.java

@@ -5,6 +5,7 @@
 import java.security.InvalidKeyException;
 import java.security.Key;
 import java.security.NoSuchAlgorithmException;
+import java.nio.charset.StandardCharsets;
 import java.util.Formatter;
 
 public class SHA256Handler {
@@ -21,7 +22,10 @@ public synchronized String createSHA256(byte[] data) throws IllegalStateExceptio
     }
 
     private Mac createSHA256Key(String authKey) throws RiskifiedError {
-        Key sk = new SecretKeySpec(authKey.getBytes(), "HmacSHA256");
+        // UTF-8, never the platform default charset: the default makes the signature depend on
+        // a JVM locale setting. Riskified tokens are hex, so UTF-8 and the reference
+        // implementation's ASCII (Riskified.SDK/Utils/HttpUtils.cs:152) agree byte for byte.
+        Key sk = new SecretKeySpec(authKey.getBytes(StandardCharsets.UTF_8), "HmacSHA256");
         Mac mac;
         try {
             mac = Mac.getInstance(sk.getAlgorithm());