From 06f52cb1f8269910eb1a8784786446747d8c19d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armando=20Rodr=C3=ADguez?= <127134616+armando-rodriguez-cko@users.noreply.github.com> Date: Mon, 9 Nov 2026 13:42:31 +0100 Subject: [PATCH 1/4] fix(client): security hardening Security hardening and input validation in request path building. Fixed path prefixes are passed one segment at a time; request URLs are unchanged. --- .../java/com/checkout/AbstractClient.java | 54 ++++++++++++++- .../checkout/ApacheHttpClientTransport.java | 19 ++++++ .../com/checkout/apm/bacs/BacsClientImpl.java | 7 +- .../checkout/apm/ideal/IdealClientImpl.java | 2 +- .../apm/previous/sepa/SepaClientImpl.java | 19 +++--- .../events/previous/EventsClientImpl.java | 2 +- .../setups/PaymentSetupsClientImpl.java | 11 +-- .../instruments/InstrumentsClientImpl.java | 7 +- .../checkout/issuing/IssuingClientImpl.java | 8 ++- .../checkout/AbstractClientBuildPathTest.java | 54 +++++++++++++++ ...acheHttpClientTransportRequestUrlTest.java | 67 +++++++++++++++++++ .../apm/ideal/IdealClientImplTest.java | 21 +++++- .../events/previous/EventsClientImplTest.java | 13 ++++ 13 files changed, 256 insertions(+), 28 deletions(-) create mode 100644 src/test/java/com/checkout/AbstractClientBuildPathTest.java create mode 100644 src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java diff --git a/src/main/java/com/checkout/AbstractClient.java b/src/main/java/com/checkout/AbstractClient.java index c9fe349ee..06ddf2032 100644 --- a/src/main/java/com/checkout/AbstractClient.java +++ b/src/main/java/com/checkout/AbstractClient.java @@ -2,6 +2,8 @@ import static com.checkout.common.CheckoutUtils.validateParams; +import java.nio.charset.StandardCharsets; + public abstract class AbstractClient { protected final ApiClient apiClient; @@ -25,8 +27,58 @@ protected SdkAuthorization sdkAuthorization() { return configuration.getSdkCredentials().getAuthorization(authorizationType); } + private static final char[] HEX = "0123456789ABCDEF".toCharArray(); + + /** + * Joins the parameters into a request path, percent-encoding each one as exactly one segment. A + * prefix of several segments must be passed as separate parameters. + * + * @param pathParams the path segments, in order + * @return the request path, relative to the base URI + * @throws CheckoutArgumentException if a parameter is null, empty, "." or ".." + */ protected static String buildPath(final String... pathParams) { - return String.join("/", pathParams); + final StringBuilder path = new StringBuilder(); + for (final String pathParam : pathParams) { + if (pathParam == null || pathParam.isEmpty() || ".".equals(pathParam) || "..".equals(pathParam)) { + throw new CheckoutArgumentException("Invalid path parameter: it must not be null, empty, '.' or '..'"); + } + if (path.length() > 0) { + path.append('/'); + } + path.append(encode(pathParam)); + } + return path.toString(); + } + + /** + * Percent-encodes a value as UTF-8, keeping only the RFC 3986 unreserved characters + * (letters, digits, "-", ".", "_" and "~"), so it can be placed in a path segment or a query value. + * + * @param value the value to encode + * @return the encoded value + * @throws CheckoutArgumentException if the value is not valid UTF-16 text (an unpaired surrogate) + */ + protected static String encode(final String value) { + for (int i = 0; i < value.length(); i++) { + final char c = value.charAt(i); + if (Character.isHighSurrogate(c) && i + 1 < value.length() && Character.isLowSurrogate(value.charAt(i + 1))) { + i++; + } else if (Character.isSurrogate(c)) { + throw new CheckoutArgumentException("Invalid parameter: it must be valid text"); + } + } + final StringBuilder encoded = new StringBuilder(value.length()); + for (final byte b : value.getBytes(StandardCharsets.UTF_8)) { + final int c = b & 0xFF; + if ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') + || c == '-' || c == '.' || c == '_' || c == '~') { + encoded.append((char) c); + } else { + encoded.append('%').append(HEX[c >> 4]).append(HEX[c & 0x0F]); + } + } + return encoded.toString(); } protected boolean isSandbox() { diff --git a/src/main/java/com/checkout/ApacheHttpClientTransport.java b/src/main/java/com/checkout/ApacheHttpClientTransport.java index c36a7a94d..686226703 100644 --- a/src/main/java/com/checkout/ApacheHttpClientTransport.java +++ b/src/main/java/com/checkout/ApacheHttpClientTransport.java @@ -62,6 +62,7 @@ class ApacheHttpClientTransport implements Transport { private static final String FILE = "file"; private static final String PURPOSE = "purpose"; private static final String PATH = "path"; + private static final String INVALID_REQUEST_PATH = "Invalid request path: a path parameter is empty, '.' or '..'"; private final URI baseUri; private final CloseableHttpClient httpClient; @@ -382,10 +383,28 @@ private HttpUriRequest buildRequest(final ClientOperation clientOperation, final } private String getRequestUrl(final String path) { + validateRequestPath(path); try { return baseUri.resolve(path).toURL().toString(); } catch (final MalformedURLException e) { throw new CheckoutException(e); } } + + /** + * Refuses a relative request path that, before any query, is empty, starts with "/", has an empty, + * "." or ".." segment, or contains a "#". + */ + private static void validateRequestPath(final String path) { + if (path == null || path.indexOf('#') >= 0) { + throw new CheckoutArgumentException(INVALID_REQUEST_PATH); + } + final int query = path.indexOf('?'); + final String pathOnly = query >= 0 ? path.substring(0, query) : path; + for (final String segment : pathOnly.split("/", -1)) { + if (segment.isEmpty() || ".".equals(segment) || "..".equals(segment)) { + throw new CheckoutArgumentException(INVALID_REQUEST_PATH); + } + } + } } diff --git a/src/main/java/com/checkout/apm/bacs/BacsClientImpl.java b/src/main/java/com/checkout/apm/bacs/BacsClientImpl.java index 2e30af33c..25b3bb403 100644 --- a/src/main/java/com/checkout/apm/bacs/BacsClientImpl.java +++ b/src/main/java/com/checkout/apm/bacs/BacsClientImpl.java @@ -12,7 +12,8 @@ public class BacsClientImpl extends AbstractClient implements BacsClient { private static final String APMS = "apms"; - private static final String BACS_NOTIFICATIONS = "bacs/notifications"; + private static final String BACS = "bacs"; + private static final String NOTIFICATIONS = "notifications"; private static final String BACS_NOTIFICATION_REQUEST = "bacsNotificationRequest"; /** @@ -26,14 +27,14 @@ public BacsClientImpl(final ApiClient apiClient, final CheckoutConfiguration con @Override public CompletableFuture sendNotification(final BacsNotificationRequest bacsNotificationRequest) { validateParams(BACS_NOTIFICATION_REQUEST, bacsNotificationRequest); - return apiClient.postAsync(buildPath(APMS, BACS_NOTIFICATIONS), sdkAuthorization(), BacsNotificationResponse.class, bacsNotificationRequest, null); + return apiClient.postAsync(buildPath(APMS, BACS, NOTIFICATIONS), sdkAuthorization(), BacsNotificationResponse.class, bacsNotificationRequest, null); } // Synchronous methods @Override public BacsNotificationResponse sendNotificationSync(final BacsNotificationRequest bacsNotificationRequest) { validateParams(BACS_NOTIFICATION_REQUEST, bacsNotificationRequest); - return apiClient.post(buildPath(APMS, BACS_NOTIFICATIONS), sdkAuthorization(), BacsNotificationResponse.class, bacsNotificationRequest, null); + return apiClient.post(buildPath(APMS, BACS, NOTIFICATIONS), sdkAuthorization(), BacsNotificationResponse.class, bacsNotificationRequest, null); } } diff --git a/src/main/java/com/checkout/apm/ideal/IdealClientImpl.java b/src/main/java/com/checkout/apm/ideal/IdealClientImpl.java index 2e844998d..f4d716e89 100644 --- a/src/main/java/com/checkout/apm/ideal/IdealClientImpl.java +++ b/src/main/java/com/checkout/apm/ideal/IdealClientImpl.java @@ -9,7 +9,7 @@ public class IdealClientImpl extends AbstractClient implements IdealClient { - private static final String IDEAL_EXTERNAL_PATH = "/ideal-external"; + private static final String IDEAL_EXTERNAL_PATH = "ideal-external"; private static final String ISSUERS = "issuers"; public IdealClientImpl(final ApiClient apiClient, final CheckoutConfiguration configuration) { diff --git a/src/main/java/com/checkout/apm/previous/sepa/SepaClientImpl.java b/src/main/java/com/checkout/apm/previous/sepa/SepaClientImpl.java index 8baa02586..11b6affde 100644 --- a/src/main/java/com/checkout/apm/previous/sepa/SepaClientImpl.java +++ b/src/main/java/com/checkout/apm/previous/sepa/SepaClientImpl.java @@ -12,7 +12,8 @@ public class SepaClientImpl extends AbstractClient implements SepaClient { private static final String APMS = "apms"; - private static final String SEPA_MANDATES = "sepa/mandates"; + private static final String SEPA = "sepa"; + private static final String MANDATES = "mandates"; private static final String PPRO = "ppro"; private static final String CANCEL = "cancel"; private static final String MANDATE_ID = "mandateId"; @@ -24,50 +25,50 @@ public SepaClientImpl(final ApiClient apiClient, final CheckoutConfiguration con @Override public CompletableFuture getMandate(final String mandateId) { validateMandateId(mandateId); - return apiClient.getAsync(buildPath(SEPA_MANDATES, mandateId), sdkAuthorization(), MandateResponse.class); + return apiClient.getAsync(buildPath(SEPA, MANDATES, mandateId), sdkAuthorization(), MandateResponse.class); } @Override public CompletableFuture cancelMandate(final String mandateId) { validateMandateId(mandateId); - return apiClient.postAsync(buildPath(SEPA_MANDATES, mandateId, CANCEL), sdkAuthorization(), SepaResource.class, null, null); + return apiClient.postAsync(buildPath(SEPA, MANDATES, mandateId, CANCEL), sdkAuthorization(), SepaResource.class, null, null); } @Override public CompletableFuture getMandateViaPPRO(final String mandateId) { validateMandateId(mandateId); - return apiClient.getAsync(buildPath(APMS, PPRO, SEPA_MANDATES, mandateId), sdkAuthorization(), MandateResponse.class); + return apiClient.getAsync(buildPath(APMS, PPRO, SEPA, MANDATES, mandateId), sdkAuthorization(), MandateResponse.class); } @Override public CompletableFuture cancelMandateViaPPRO(final String mandateId) { validateMandateId(mandateId); - return apiClient.postAsync(buildPath(APMS, PPRO, SEPA_MANDATES, mandateId, CANCEL), sdkAuthorization(), SepaResource.class, null, null); + return apiClient.postAsync(buildPath(APMS, PPRO, SEPA, MANDATES, mandateId, CANCEL), sdkAuthorization(), SepaResource.class, null, null); } // Synchronous methods @Override public MandateResponse getMandateSync(final String mandateId) { validateMandateId(mandateId); - return apiClient.get(buildPath(SEPA_MANDATES, mandateId), sdkAuthorization(), MandateResponse.class); + return apiClient.get(buildPath(SEPA, MANDATES, mandateId), sdkAuthorization(), MandateResponse.class); } @Override public SepaResource cancelMandateSync(final String mandateId) { validateMandateId(mandateId); - return apiClient.post(buildPath(SEPA_MANDATES, mandateId, CANCEL), sdkAuthorization(), SepaResource.class, null, null); + return apiClient.post(buildPath(SEPA, MANDATES, mandateId, CANCEL), sdkAuthorization(), SepaResource.class, null, null); } @Override public MandateResponse getMandateViaPPROSync(final String mandateId) { validateMandateId(mandateId); - return apiClient.get(buildPath(APMS, PPRO, SEPA_MANDATES, mandateId), sdkAuthorization(), MandateResponse.class); + return apiClient.get(buildPath(APMS, PPRO, SEPA, MANDATES, mandateId), sdkAuthorization(), MandateResponse.class); } @Override public SepaResource cancelMandateViaPPROSync(final String mandateId) { validateMandateId(mandateId); - return apiClient.post(buildPath(APMS, PPRO, SEPA_MANDATES, mandateId, CANCEL), sdkAuthorization(), SepaResource.class, null, null); + return apiClient.post(buildPath(APMS, PPRO, SEPA, MANDATES, mandateId, CANCEL), sdkAuthorization(), SepaResource.class, null, null); } // Common methods diff --git a/src/main/java/com/checkout/events/previous/EventsClientImpl.java b/src/main/java/com/checkout/events/previous/EventsClientImpl.java index ea46e8d97..58fab061f 100644 --- a/src/main/java/com/checkout/events/previous/EventsClientImpl.java +++ b/src/main/java/com/checkout/events/previous/EventsClientImpl.java @@ -105,7 +105,7 @@ public EmptyResponse retryAllWebhooksSync(final String eventId) { private StringBuilder buildEventTypesPath(final String version) { final StringBuilder path = new StringBuilder(EVENT_TYPES_PATH); if (version != null) { - path.append("?version=").append(version); + path.append("?version=").append(encode(version)); } return path; } diff --git a/src/main/java/com/checkout/handlepaymentsandpayouts/setups/PaymentSetupsClientImpl.java b/src/main/java/com/checkout/handlepaymentsandpayouts/setups/PaymentSetupsClientImpl.java index c21e339bd..62b19efa6 100644 --- a/src/main/java/com/checkout/handlepaymentsandpayouts/setups/PaymentSetupsClientImpl.java +++ b/src/main/java/com/checkout/handlepaymentsandpayouts/setups/PaymentSetupsClientImpl.java @@ -17,7 +17,8 @@ */ public class PaymentSetupsClientImpl extends AbstractClient implements PaymentSetupsClient { - private static final String PAYMENT_SETUPS_PATH = "payments/setups"; + private static final String PAYMENTS_PATH = "payments"; + private static final String SETUPS_PATH = "setups"; private static final String CONFIRM_PATH = "confirm"; public PaymentSetupsClientImpl(final ApiClient apiClient, final CheckoutConfiguration configuration) { @@ -34,7 +35,7 @@ public PaymentSetupsClientImpl(final ApiClient apiClient, final CheckoutConfigur public CompletableFuture createPaymentSetup( final PaymentSetupsRequest paymentSetupsRequest) { validateParams("paymentSetupsRequest", paymentSetupsRequest); - return apiClient.postAsync(PAYMENT_SETUPS_PATH, sdkAuthorization(), PaymentSetupsResponse.class, + return apiClient.postAsync(buildPath(PAYMENTS_PATH, SETUPS_PATH), sdkAuthorization(), PaymentSetupsResponse.class, paymentSetupsRequest, null); } @@ -49,7 +50,7 @@ public CompletableFuture createPaymentSetup( public CompletableFuture updatePaymentSetup(final String id, final PaymentSetupsRequest paymentSetupsRequest) { validateParams("id", id, "paymentSetupsRequest", paymentSetupsRequest); - return apiClient.putAsync(buildPath(PAYMENT_SETUPS_PATH, id), sdkAuthorization(), PaymentSetupsResponse.class, + return apiClient.putAsync(buildPath(PAYMENTS_PATH, SETUPS_PATH, id), sdkAuthorization(), PaymentSetupsResponse.class, paymentSetupsRequest); } @@ -62,7 +63,7 @@ public CompletableFuture updatePaymentSetup(final String @Override public CompletableFuture getPaymentSetup(final String id) { validateParams("id", id); - return apiClient.getAsync(buildPath(PAYMENT_SETUPS_PATH, id), sdkAuthorization(), PaymentSetupsResponse.class); + return apiClient.getAsync(buildPath(PAYMENTS_PATH, SETUPS_PATH, id), sdkAuthorization(), PaymentSetupsResponse.class); } /** @@ -76,7 +77,7 @@ public CompletableFuture getPaymentSetup(final String id) public CompletableFuture confirmPaymentSetup(final String id, final String paymentMethodName) { validateParams("id", id, "paymentMethodName", paymentMethodName); - return apiClient.postAsync(buildPath(PAYMENT_SETUPS_PATH, id, CONFIRM_PATH, paymentMethodName), + return apiClient.postAsync(buildPath(PAYMENTS_PATH, SETUPS_PATH, id, CONFIRM_PATH, paymentMethodName), sdkAuthorization(), PaymentSetupsConfirmResponse.class, null, null); } } \ No newline at end of file diff --git a/src/main/java/com/checkout/instruments/InstrumentsClientImpl.java b/src/main/java/com/checkout/instruments/InstrumentsClientImpl.java index 48684137d..d533e0338 100644 --- a/src/main/java/com/checkout/instruments/InstrumentsClientImpl.java +++ b/src/main/java/com/checkout/instruments/InstrumentsClientImpl.java @@ -26,7 +26,8 @@ public class InstrumentsClientImpl extends AbstractClient implements InstrumentsClient { private static final String INSTRUMENTS_PATH = "instruments"; - private static final String VALIDATION_PATH = "validation/bank-accounts"; + private static final String VALIDATION_PATH = "validation"; + private static final String BANK_ACCOUNTS_PATH = "bank-accounts"; private static final String REVOKE_PATH = "revoke"; private static final Type CREATE_TYPE = TypeToken.get(CreateInstrumentResponse.class).getType(); @@ -70,7 +71,7 @@ public CompletableFuture revoke(final String instrumentId) { @Override public CompletableFuture getBankAccountFieldFormatting(final CountryCode country, final Currency currency, final BankAccountFieldQuery query) { validateBankAccountFieldParams(country, currency, query); - return apiClient.queryAsync(buildPath(VALIDATION_PATH, country.name(), currency.name()), sdkAuthorization(SdkAuthorizationType.OAUTH), query, BankAccountFieldResponse.class); + return apiClient.queryAsync(buildPath(VALIDATION_PATH, BANK_ACCOUNTS_PATH, country.name(), currency.name()), sdkAuthorization(SdkAuthorizationType.OAUTH), query, BankAccountFieldResponse.class); } // Synchronous methods @@ -107,7 +108,7 @@ public EmptyResponse revokeSync(final String instrumentId) { @Override public BankAccountFieldResponse getBankAccountFieldFormattingSync(final CountryCode country, final Currency currency, final BankAccountFieldQuery query) { validateBankAccountFieldParams(country, currency, query); - return apiClient.query(buildPath(VALIDATION_PATH, country.name(), currency.name()), sdkAuthorization(SdkAuthorizationType.OAUTH), query, BankAccountFieldResponse.class); + return apiClient.query(buildPath(VALIDATION_PATH, BANK_ACCOUNTS_PATH, country.name(), currency.name()), sdkAuthorization(SdkAuthorizationType.OAUTH), query, BankAccountFieldResponse.class); } // Common methods diff --git a/src/main/java/com/checkout/issuing/IssuingClientImpl.java b/src/main/java/com/checkout/issuing/IssuingClientImpl.java index 7177ad35e..1d74d1dd8 100644 --- a/src/main/java/com/checkout/issuing/IssuingClientImpl.java +++ b/src/main/java/com/checkout/issuing/IssuingClientImpl.java @@ -110,7 +110,9 @@ public class IssuingClientImpl extends AbstractClient implements IssuingClient { private static final String RENEW_PATH = "renew"; - private static final String ACCESS_TOKEN_PATH = "access/connect/token"; + private static final String ACCESS_PATH = "access"; + private static final String CONNECT_PATH = "connect"; + private static final String TOKEN_PATH = "token"; private static final String DISPUTES_PATH = "disputes"; private static final String TRANSACTIONS_PATH = "transactions"; @@ -134,7 +136,7 @@ public CompletableFuture requestCardholderAccessT final UrlEncodedFormEntity form = createFormUrlEncodedContent(cardholderAccessTokenRequest); return apiClient.postAsync( - buildPath(ISSUING_PATH, ACCESS_TOKEN_PATH), + buildPath(ISSUING_PATH, ACCESS_PATH, CONNECT_PATH, TOKEN_PATH), sdkAuthorization(), CardholderAccessTokenResponse.class, form, @@ -727,7 +729,7 @@ public CardholderAccessTokenResponse requestCardholderAccessTokenSync(final Card UrlEncodedFormEntity form = createFormUrlEncodedContent(cardholderAccessTokenRequest); return apiClient.post( - buildPath(ISSUING_PATH, ACCESS_TOKEN_PATH), + buildPath(ISSUING_PATH, ACCESS_PATH, CONNECT_PATH, TOKEN_PATH), sdkAuthorization(), CardholderAccessTokenResponse.class, form, diff --git a/src/test/java/com/checkout/AbstractClientBuildPathTest.java b/src/test/java/com/checkout/AbstractClientBuildPathTest.java new file mode 100644 index 000000000..69dba3e39 --- /dev/null +++ b/src/test/java/com/checkout/AbstractClientBuildPathTest.java @@ -0,0 +1,54 @@ +package com.checkout; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.NullSource; +import org.junit.jupiter.params.provider.ValueSource; + +class AbstractClientBuildPathTest { + + private static Stream encodedValues() { + return Stream.of( + Arguments.of("pay_123", "payments/pay_123"), + Arguments.of("pay_1/refunds", "payments/pay_1%2Frefunds"), + Arguments.of("../payments/pay_1", "payments/..%2Fpayments%2Fpay_1"), + Arguments.of("pay_1?x=1", "payments/pay_1%3Fx%3D1"), + Arguments.of("pay_1#x", "payments/pay_1%23x"), + Arguments.of("pay_1%2Frefunds", "payments/pay_1%252Frefunds"), + Arguments.of("pay_1\\refunds", "payments/pay_1%5Crefunds"), + Arguments.of("my secret", "payments/my%20secret"), + Arguments.of("pay_1\r\nX: y", "payments/pay_1%0D%0AX%3A%20y"), + Arguments.of("pay_1\u0000", "payments/pay_1%00"), + Arguments.of("clé", "payments/cl%C3%A9"), + Arguments.of("a.b-c_d~e", "payments/a.b-c_d~e"), + Arguments.of("...", "payments/..."), + Arguments.of("\ud83d\ude00", "payments/%F0%9F%98%80") + ); + } + + @ParameterizedTest + @MethodSource("encodedValues") + void shouldEncodeEachPathParameterAsOneSegment(final String value, final String expected) { + assertEquals(expected, AbstractClient.buildPath("payments", value)); + } + + @Test + void shouldKeepPlainPathsUnchanged() { + assertEquals("disputes/dsp_123/evidence", AbstractClient.buildPath("disputes", "dsp_123", "evidence")); + } + + @ParameterizedTest + @NullSource + @ValueSource(strings = {"", ".", "..", "\ud800", "a\udc00b", "\ud83d"}) + void shouldRefuseEmptyAndDotPathParameters(final String value) { + assertThrows(CheckoutArgumentException.class, () -> AbstractClient.buildPath("payments", value, "refunds")); + } + +} diff --git a/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java b/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java new file mode 100644 index 000000000..224524684 --- /dev/null +++ b/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java @@ -0,0 +1,67 @@ +package com.checkout; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URI; +import java.util.concurrent.Executors; + +import org.apache.http.impl.client.HttpClientBuilder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +class ApacheHttpClientTransportRequestUrlTest { + + private ApacheHttpClientTransport transport; + + @BeforeEach + void setUp() { + transport = new ApacheHttpClientTransport( + URI.create("https://api.sandbox.checkout.com/"), + HttpClientBuilder.create(), + Executors.newSingleThreadExecutor(), + new DefaultTransportConfiguration(), + mock(CheckoutConfiguration.class)); + } + + private String requestUrl(final String path) throws Throwable { + final Method method = ApacheHttpClientTransport.class.getDeclaredMethod("getRequestUrl", String.class); + method.setAccessible(true); + try { + return (String) method.invoke(transport, path); + } catch (final InvocationTargetException e) { + throw e.getCause(); + } + } + + @Test + void shouldResolveEncodedPathsUnchanged() throws Throwable { + assertEquals("https://api.sandbox.checkout.com/payments/pay_1", requestUrl("payments/pay_1")); + assertEquals("https://api.sandbox.checkout.com/payments/pay_1%2Frefunds", requestUrl("payments/pay_1%2Frefunds")); + assertEquals("https://api.sandbox.checkout.com/event-types?version=v2", requestUrl("event-types?version=v2")); + assertEquals("https://api.sandbox.checkout.com/ideal-external/issuers", requestUrl("ideal-external/issuers")); + } + + @ParameterizedTest + @ValueSource(strings = { + "", + "payments/../workflows/wf_1", + "payments/./pay_1", + "payments/..", + "payments//refunds", + "payments/pay_1/", + "/payments/pay_1", + "//example.com/payments", + "payments/pay_1#refunds", + "payments/pay_1/..?x=1" + }) + void shouldRefuseUnsafeRequestPaths(final String path) { + assertThrows(CheckoutArgumentException.class, () -> requestUrl(path)); + } + +} diff --git a/src/test/java/com/checkout/apm/ideal/IdealClientImplTest.java b/src/test/java/com/checkout/apm/ideal/IdealClientImplTest.java index c04e8cdc9..3b05d9c59 100644 --- a/src/test/java/com/checkout/apm/ideal/IdealClientImplTest.java +++ b/src/test/java/com/checkout/apm/ideal/IdealClientImplTest.java @@ -45,10 +45,27 @@ void setUp() { this.idealClient = new IdealClientImpl(apiClient, configuration); } + @Test + void shouldGetInfo() throws ExecutionException, InterruptedException { + final IdealInfo idealInfo = new IdealInfo(); + when(apiClient.getAsync("ideal-external", authorization, IdealInfo.class)) + .thenReturn(CompletableFuture.completedFuture(idealInfo)); + + assertEquals(idealInfo, idealClient.getInfo().get()); + } + + @Test + void shouldGetInfoSync() { + final IdealInfo idealInfo = new IdealInfo(); + when(apiClient.get("ideal-external", authorization, IdealInfo.class)).thenReturn(idealInfo); + + assertEquals(idealInfo, idealClient.getInfoSync()); + } + @Test void shouldGetIssuers() throws ExecutionException, InterruptedException { - when(apiClient.getAsync("/ideal-external/issuers", authorization, IssuerResponse.class)) + when(apiClient.getAsync("ideal-external/issuers", authorization, IssuerResponse.class)) .thenReturn(CompletableFuture.completedFuture(issuerResponse)); final CompletableFuture future = idealClient.getIssuers(); @@ -61,7 +78,7 @@ void shouldGetIssuers() throws ExecutionException, InterruptedException { @Test void shouldGetIssuersSync() throws ExecutionException, InterruptedException { - when(apiClient.get("/ideal-external/issuers", authorization, IssuerResponse.class)) + when(apiClient.get("ideal-external/issuers", authorization, IssuerResponse.class)) .thenReturn(issuerResponse); final IssuerResponse response = idealClient.getIssuersSync(); diff --git a/src/test/java/com/checkout/events/previous/EventsClientImplTest.java b/src/test/java/com/checkout/events/previous/EventsClientImplTest.java index ffa25d346..9721de830 100644 --- a/src/test/java/com/checkout/events/previous/EventsClientImplTest.java +++ b/src/test/java/com/checkout/events/previous/EventsClientImplTest.java @@ -80,6 +80,19 @@ void shouldRetrieveAllEventTypes_withVersion() throws ExecutionException, Interr validateItemsResponse(response, eventTypes.get()); } + @Test + void shouldRetrieveAllEventTypes_encodesVersion() throws ExecutionException, InterruptedException { + final ItemsResponse response = createMockItemsResponse(); + + createMockSdkCredentials(); + when(apiClient.getAsync("event-types?version=v2%26x%3D1%23y", authorization, EVENT_TYPES_TYPE)) + .thenReturn(CompletableFuture.completedFuture(response)); + + final CompletableFuture> eventTypes = eventsClient.retrieveAllEventTypes("v2&x=1#y"); + + validateItemsResponse(response, eventTypes.get()); + } + @Test void shouldRetrieveEvents() throws ExecutionException, InterruptedException { final RetrieveEventsRequest retrieveEventsRequest = createMockRetrieveEventsRequest(); From fb76572604d1873ed3cf3ca35c76709beb12690e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armando=20Rodr=C3=ADguez?= <127134616+armando-rodriguez-cko@users.noreply.github.com> Date: Mon, 9 Nov 2026 22:10:32 +0100 Subject: [PATCH 2/4] fix(client): security hardening in request building Input validation of the request path on every HTTP operation; the resolved URL must stay under the base URI. Query parameters are always sent apart from the path, with new Type overloads for query. --- .../java/com/checkout/AbstractClient.java | 58 +++--- .../checkout/ApacheHttpClientTransport.java | 55 ++++-- src/main/java/com/checkout/ApiClient.java | 24 +++ src/main/java/com/checkout/ApiClientImpl.java | 16 ++ .../events/previous/EventsClientImpl.java | 20 +-- .../checkout/AbstractClientBuildPathTest.java | 2 +- ...acheHttpClientTransportRequestUrlTest.java | 168 +++++++++++++++--- .../events/previous/EventsClientImplTest.java | 12 +- 8 files changed, 274 insertions(+), 81 deletions(-) diff --git a/src/main/java/com/checkout/AbstractClient.java b/src/main/java/com/checkout/AbstractClient.java index 06ddf2032..774fcb10f 100644 --- a/src/main/java/com/checkout/AbstractClient.java +++ b/src/main/java/com/checkout/AbstractClient.java @@ -6,6 +6,10 @@ public abstract class AbstractClient { + private static final char[] HEX = "0123456789ABCDEF".toCharArray(); + private static final String INVALID_PATH_PARAMETER = + "Invalid path parameter: it must be valid text, not null, empty, '.' or '..'"; + protected final ApiClient apiClient; protected final CheckoutConfiguration configuration; private final SdkAuthorizationType authorizationType; @@ -27,52 +31,39 @@ protected SdkAuthorization sdkAuthorization() { return configuration.getSdkCredentials().getAuthorization(authorizationType); } - private static final char[] HEX = "0123456789ABCDEF".toCharArray(); - /** * Joins the parameters into a request path, percent-encoding each one as exactly one segment. A * prefix of several segments must be passed as separate parameters. * * @param pathParams the path segments, in order * @return the request path, relative to the base URI - * @throws CheckoutArgumentException if a parameter is null, empty, "." or ".." + * @throws CheckoutArgumentException if a parameter is null, empty, "." or "..", or is not valid + * UTF-16 text (an unpaired surrogate) */ protected static String buildPath(final String... pathParams) { final StringBuilder path = new StringBuilder(); for (final String pathParam : pathParams) { - if (pathParam == null || pathParam.isEmpty() || ".".equals(pathParam) || "..".equals(pathParam)) { - throw new CheckoutArgumentException("Invalid path parameter: it must not be null, empty, '.' or '..'"); - } if (path.length() > 0) { path.append('/'); } - path.append(encode(pathParam)); + path.append(encodePathSegment(pathParam)); } return path.toString(); } /** - * Percent-encodes a value as UTF-8, keeping only the RFC 3986 unreserved characters - * (letters, digits, "-", ".", "_" and "~"), so it can be placed in a path segment or a query value. - * - * @param value the value to encode - * @return the encoded value - * @throws CheckoutArgumentException if the value is not valid UTF-16 text (an unpaired surrogate) + * Percent-encodes a path parameter as UTF-8, keeping only the RFC 3986 unreserved characters + * (letters, digits, "-", ".", "_" and "~"), so the result is exactly one path segment. */ - protected static String encode(final String value) { - for (int i = 0; i < value.length(); i++) { - final char c = value.charAt(i); - if (Character.isHighSurrogate(c) && i + 1 < value.length() && Character.isLowSurrogate(value.charAt(i + 1))) { - i++; - } else if (Character.isSurrogate(c)) { - throw new CheckoutArgumentException("Invalid parameter: it must be valid text"); - } + private static String encodePathSegment(final String pathParam) { + if (pathParam == null || pathParam.isEmpty() || ".".equals(pathParam) || "..".equals(pathParam) + || hasUnpairedSurrogate(pathParam)) { + throw new CheckoutArgumentException(INVALID_PATH_PARAMETER); } - final StringBuilder encoded = new StringBuilder(value.length()); - for (final byte b : value.getBytes(StandardCharsets.UTF_8)) { + final StringBuilder encoded = new StringBuilder(pathParam.length()); + for (final byte b : pathParam.getBytes(StandardCharsets.UTF_8)) { final int c = b & 0xFF; - if ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') - || c == '-' || c == '.' || c == '_' || c == '~') { + if (isUnreserved(c)) { encoded.append((char) c); } else { encoded.append('%').append(HEX[c >> 4]).append(HEX[c & 0x0F]); @@ -81,6 +72,23 @@ protected static String encode(final String value) { return encoded.toString(); } + private static boolean isUnreserved(final int c) { + return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') + || c == '-' || c == '.' || c == '_' || c == '~'; + } + + private static boolean hasUnpairedSurrogate(final String value) { + for (int i = 0; i < value.length(); i++) { + final char c = value.charAt(i); + if (Character.isHighSurrogate(c) && i + 1 < value.length() && Character.isLowSurrogate(value.charAt(i + 1))) { + i++; + } else if (Character.isSurrogate(c)) { + return true; + } + } + return false; + } + protected boolean isSandbox() { return configuration.getEnvironment().isSandbox(); } diff --git a/src/main/java/com/checkout/ApacheHttpClientTransport.java b/src/main/java/com/checkout/ApacheHttpClientTransport.java index 686226703..a06423948 100644 --- a/src/main/java/com/checkout/ApacheHttpClientTransport.java +++ b/src/main/java/com/checkout/ApacheHttpClientTransport.java @@ -62,7 +62,7 @@ class ApacheHttpClientTransport implements Transport { private static final String FILE = "file"; private static final String PURPOSE = "purpose"; private static final String PATH = "path"; - private static final String INVALID_REQUEST_PATH = "Invalid request path: a path parameter is empty, '.' or '..'"; + private static final String INVALID_REQUEST_PATH = "Invalid request path"; private final URI baseUri; private final CloseableHttpClient httpClient; @@ -382,29 +382,60 @@ private HttpUriRequest buildRequest(final ClientOperation clientOperation, final return request; } - private String getRequestUrl(final String path) { + /** + * Resolves a request path against the base URI. The path must be relative and made of non-empty + * segments, none of them "." or ".." (also when percent-encoded or followed by ";"), with no "?" or + * "#": query parameters are added separately. The resolved URL must keep the scheme, host, port and + * base path of the base URI. + * + * @param path the request path, as built by {@code AbstractClient.buildPath} + * @return the absolute request URL + * @throws CheckoutArgumentException if the path is invalid or resolves outside the base URI + */ + String getRequestUrl(final String path) { validateRequestPath(path); + final URI resolved = baseUri.resolve(path); + if (!isUnderBaseUri(resolved)) { + throw new CheckoutArgumentException(INVALID_REQUEST_PATH); + } try { - return baseUri.resolve(path).toURL().toString(); + return resolved.toURL().toString(); } catch (final MalformedURLException e) { throw new CheckoutException(e); } } - /** - * Refuses a relative request path that, before any query, is empty, starts with "/", has an empty, - * "." or ".." segment, or contains a "#". - */ private static void validateRequestPath(final String path) { - if (path == null || path.indexOf('#') >= 0) { + if (path == null || path.isEmpty() || path.indexOf('?') >= 0 || path.indexOf('#') >= 0) { throw new CheckoutArgumentException(INVALID_REQUEST_PATH); } - final int query = path.indexOf('?'); - final String pathOnly = query >= 0 ? path.substring(0, query) : path; - for (final String segment : pathOnly.split("/", -1)) { - if (segment.isEmpty() || ".".equals(segment) || "..".equals(segment)) { + for (final String segment : path.split("/", -1)) { + if (isEmptyOrDotSegment(segment)) { throw new CheckoutArgumentException(INVALID_REQUEST_PATH); } } } + + private static boolean isEmptyOrDotSegment(final String segment) { + final int parameters = segment.indexOf(';'); + final String name = (parameters >= 0 ? segment.substring(0, parameters) : segment).replaceAll("(?i)%2e", "."); + return name.isEmpty() || ".".equals(name) || "..".equals(name); + } + + /** + * True when the URL has the scheme, host and port of the base URI and a path under the base URI's + * directory, the part of its path that {@link URI#resolve(String)} keeps. + */ + private boolean isUnderBaseUri(final URI resolved) { + final String basePath = baseUri.getRawPath() == null ? "" : baseUri.getRawPath(); + final String baseDirectory = basePath.substring(0, basePath.lastIndexOf('/') + 1); + return baseUri.getScheme().equalsIgnoreCase(resolved.getScheme()) + && baseUri.getHost() != null + && baseUri.getHost().equalsIgnoreCase(resolved.getHost()) + && baseUri.getPort() == resolved.getPort() + && resolved.getRawPath() != null + && resolved.getRawPath().startsWith(baseDirectory) + && resolved.getRawQuery() == null + && resolved.getRawFragment() == null; + } } diff --git a/src/main/java/com/checkout/ApiClient.java b/src/main/java/com/checkout/ApiClient.java index 5e50a7e7b..50e3ebc2d 100644 --- a/src/main/java/com/checkout/ApiClient.java +++ b/src/main/java/com/checkout/ApiClient.java @@ -38,6 +38,18 @@ public interface ApiClient { CompletableFuture queryAsync(String path, SdkAuthorization authorization, Object filter, Class responseType); + /** + * Sends a GET request with the filter's fields as query parameters, for a generic response type. + * + * @param path the request path, relative to the base URI + * @param authorization the authorization to send + * @param filter the object whose fields become the query parameters; null fields are left out + * @param responseType the response type, for example a {@code TypeToken} of {@code ItemsResponse} + * @param the response type + * @return the response + */ + CompletableFuture queryAsync(String path, SdkAuthorization authorization, Object filter, Type responseType); + CompletableFuture queryCsvContentAsync(String path, SdkAuthorization authorization, Object filter, String targetFile); CompletableFuture submitFileAsync(String path, SdkAuthorization authorization, AbstractFileRequest request, Class responseType); @@ -73,6 +85,18 @@ public interface ApiClient { T query(String path, SdkAuthorization authorization, Object filter, Class responseType); + /** + * Synchronous version of {@link #queryAsync(String, SdkAuthorization, Object, Type)}. + * + * @param path the request path, relative to the base URI + * @param authorization the authorization to send + * @param filter the object whose fields become the query parameters; null fields are left out + * @param responseType the response type, for example a {@code TypeToken} of {@code ItemsResponse} + * @param the response type + * @return the response + */ + T query(String path, SdkAuthorization authorization, Object filter, Type responseType); + ContentResponse queryCsvContent(String path, SdkAuthorization authorization, Object filter, String targetFile); T submitFile(String path, SdkAuthorization authorization, AbstractFileRequest request, Class responseType); diff --git a/src/main/java/com/checkout/ApiClientImpl.java b/src/main/java/com/checkout/ApiClientImpl.java index 0478c1952..847477dc0 100644 --- a/src/main/java/com/checkout/ApiClientImpl.java +++ b/src/main/java/com/checkout/ApiClientImpl.java @@ -180,6 +180,14 @@ public CompletableFuture queryAsync(final String pat final SdkAuthorization authorization, final Object filter, final Class responseType) { + return queryAsync(path, authorization, filter, (Type) responseType); + } + + @Override + public CompletableFuture queryAsync(final String path, + final SdkAuthorization authorization, + final Object filter, + final Type responseType) { validateParams(PATH, path, AUTHORIZATION, authorization, "filter", filter); return executeAsyncOrSync( () -> query(path, authorization, filter, responseType), @@ -436,6 +444,14 @@ public T query(final String path, final SdkAuthorization authorization, final Object filter, final Class responseType) { + return query(path, authorization, filter, (Type) responseType); + } + + @Override + public T query(final String path, + final SdkAuthorization authorization, + final Object filter, + final Type responseType) { validateParams(PATH, path, AUTHORIZATION, authorization, "filter", filter); final Map params = serializer.fromJson(serializer.toJson(filter), new TypeToken>() { diff --git a/src/main/java/com/checkout/events/previous/EventsClientImpl.java b/src/main/java/com/checkout/events/previous/EventsClientImpl.java index 58fab061f..d91e88f65 100644 --- a/src/main/java/com/checkout/events/previous/EventsClientImpl.java +++ b/src/main/java/com/checkout/events/previous/EventsClientImpl.java @@ -13,6 +13,8 @@ import com.checkout.SdkAuthorizationType; import com.google.gson.reflect.TypeToken; +import lombok.Value; + public class EventsClientImpl extends AbstractClient implements EventsClient { private static final String EVENT_TYPES_PATH = "event-types"; @@ -30,8 +32,7 @@ public EventsClientImpl(final ApiClient apiClient, final CheckoutConfiguration c @Override public CompletableFuture> retrieveAllEventTypes(final String version) { - final StringBuilder path = buildEventTypesPath(version); - return apiClient.getAsync(path.toString(), sdkAuthorization(), EVENT_TYPES_TYPE); + return apiClient.queryAsync(EVENT_TYPES_PATH, sdkAuthorization(), new EventTypesQuery(version), EVENT_TYPES_TYPE); } @Override @@ -67,8 +68,7 @@ public CompletableFuture retryAllWebhooks(final String eventId) { // Synchronous methods @Override public ItemsResponse retrieveAllEventTypesSync(final String version) { - final StringBuilder path = buildEventTypesPath(version); - return apiClient.get(path.toString(), sdkAuthorization(), EVENT_TYPES_TYPE); + return apiClient.query(EVENT_TYPES_PATH, sdkAuthorization(), new EventTypesQuery(version), EVENT_TYPES_TYPE); } @Override @@ -102,12 +102,12 @@ public EmptyResponse retryAllWebhooksSync(final String eventId) { } // Common methods - private StringBuilder buildEventTypesPath(final String version) { - final StringBuilder path = new StringBuilder(EVENT_TYPES_PATH); - if (version != null) { - path.append("?version=").append(encode(version)); - } - return path; + /** + * Query of the event types request: {@code version} is sent as a query parameter, and left out when null. + */ + @Value + static class EventTypesQuery { + String version; } protected void validateRetrieveEventsRequest(final RetrieveEventsRequest retrieveEventsRequest) { diff --git a/src/test/java/com/checkout/AbstractClientBuildPathTest.java b/src/test/java/com/checkout/AbstractClientBuildPathTest.java index 69dba3e39..02a85f768 100644 --- a/src/test/java/com/checkout/AbstractClientBuildPathTest.java +++ b/src/test/java/com/checkout/AbstractClientBuildPathTest.java @@ -47,7 +47,7 @@ void shouldKeepPlainPathsUnchanged() { @ParameterizedTest @NullSource @ValueSource(strings = {"", ".", "..", "\ud800", "a\udc00b", "\ud83d"}) - void shouldRefuseEmptyAndDotPathParameters(final String value) { + void shouldRefuseInvalidPathParameters(final String value) { assertThrows(CheckoutArgumentException.class, () -> AbstractClient.buildPath("payments", value, "refunds")); } diff --git a/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java b/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java index 224524684..be2fc52dd 100644 --- a/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java +++ b/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java @@ -1,67 +1,181 @@ package com.checkout; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; +import java.io.File; +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CompletionException; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import org.apache.http.entity.ContentType; import org.apache.http.impl.client.HttpClientBuilder; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.NullSource; import org.junit.jupiter.params.provider.ValueSource; +import com.checkout.common.FilePurpose; +import com.checkout.common.FileRequest; +import com.sun.net.httpserver.HttpServer; + class ApacheHttpClientTransportRequestUrlTest { - private ApacheHttpClientTransport transport; + private static final URI SANDBOX = URI.create("https://api.sandbox.checkout.com/"); + private static final String VALID_PATH = "payments/pay_123"; + private static final String DOT_SEGMENT_PATH = "payments/../workflows/wf_1"; + + private final SdkAuthorization authorization = new SdkAuthorization(PlatformType.DEFAULT, "sk_test"); + private final List receivedPaths = Collections.synchronizedList(new ArrayList<>()); + private final List receivedQueries = Collections.synchronizedList(new ArrayList<>()); + private final List executors = new ArrayList<>(); + + private HttpServer server; + + @TempDir + File tempDir; @BeforeEach - void setUp() { - transport = new ApacheHttpClientTransport( - URI.create("https://api.sandbox.checkout.com/"), - HttpClientBuilder.create(), - Executors.newSingleThreadExecutor(), - new DefaultTransportConfiguration(), - mock(CheckoutConfiguration.class)); + void startServer() throws IOException { + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/", exchange -> { + receivedPaths.add(exchange.getRequestURI().getRawPath()); + receivedQueries.add(String.valueOf(exchange.getRequestURI().getRawQuery())); + final byte[] body = "{}".getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, body.length); + try (OutputStream out = exchange.getResponseBody()) { + out.write(body); + } + }); + server.start(); + } + + @AfterEach + void stopServerAndExecutors() { + server.stop(0); + executors.forEach(ExecutorService::shutdownNow); + } + + private ApacheHttpClientTransport transport(final URI baseUri) { + final ExecutorService executor = Executors.newSingleThreadExecutor(); + executors.add(executor); + return new ApacheHttpClientTransport(baseUri, HttpClientBuilder.create(), executor, + new DefaultTransportConfiguration(), mock(CheckoutConfiguration.class)); } - private String requestUrl(final String path) throws Throwable { - final Method method = ApacheHttpClientTransport.class.getDeclaredMethod("getRequestUrl", String.class); - method.setAccessible(true); - try { - return (String) method.invoke(transport, path); - } catch (final InvocationTargetException e) { - throw e.getCause(); - } + private ApacheHttpClientTransport localTransport() { + return transport(URI.create("http://127.0.0.1:" + server.getAddress().getPort() + "/")); } @Test - void shouldResolveEncodedPathsUnchanged() throws Throwable { - assertEquals("https://api.sandbox.checkout.com/payments/pay_1", requestUrl("payments/pay_1")); - assertEquals("https://api.sandbox.checkout.com/payments/pay_1%2Frefunds", requestUrl("payments/pay_1%2Frefunds")); - assertEquals("https://api.sandbox.checkout.com/event-types?version=v2", requestUrl("event-types?version=v2")); - assertEquals("https://api.sandbox.checkout.com/ideal-external/issuers", requestUrl("ideal-external/issuers")); + void shouldResolveValidRequestPaths() { + final ApacheHttpClientTransport transport = transport(SANDBOX); + + assertEquals("https://api.sandbox.checkout.com/payments/pay_123", transport.getRequestUrl(VALID_PATH)); + assertEquals("https://api.sandbox.checkout.com/payments/pay_1%2Frefunds", transport.getRequestUrl("payments/pay_1%2Frefunds")); + assertEquals("https://api.sandbox.checkout.com/payments/...", transport.getRequestUrl("payments/...")); + assertEquals("https://api.sandbox.checkout.com/ideal-external/issuers", transport.getRequestUrl("ideal-external/issuers")); + } + + @Test + void shouldResolveUnderCustomBaseUris() { + assertEquals("https://proxy.example/checkout/payments/pay_123", + transport(URI.create("https://proxy.example/checkout/")).getRequestUrl(VALID_PATH)); + // Without a trailing slash, URI.resolve replaces the last base segment, as it always has. + assertEquals("https://proxy.example/payments/pay_123", + transport(URI.create("https://proxy.example/checkout")).getRequestUrl(VALID_PATH)); } @ParameterizedTest + @NullSource @ValueSource(strings = { "", - "payments/../workflows/wf_1", + DOT_SEGMENT_PATH, "payments/./pay_1", "payments/..", + "payments/%2e%2e/workflows/wf_1", + "payments/%2E/pay_1", + "payments/..;x/refunds", "payments//refunds", "payments/pay_1/", "/payments/pay_1", "//example.com/payments", + "payments/pay_1/refunds?", + "event-types?version=v2", "payments/pay_1#refunds", - "payments/pay_1/..?x=1" + "https:evil.example/payments", + "https://evil.example/payments", + "mailto:x" }) - void shouldRefuseUnsafeRequestPaths(final String path) { - assertThrows(CheckoutArgumentException.class, () -> requestUrl(path)); + void shouldRefuseInvalidRequestPaths(final String path) { + assertThrows(CheckoutArgumentException.class, () -> transport(SANDBOX).getRequestUrl(path)); + } + + @ParameterizedTest + @EnumSource(ClientOperation.class) + void shouldSendValidPathsAndRefuseDotSegmentsOnEveryOperation(final ClientOperation operation) { + final ApacheHttpClientTransport transport = localTransport(); + + transport.invokeSync(operation, VALID_PATH, authorization, null, null, Collections.emptyMap()); + transport.invoke(operation, VALID_PATH, authorization, null, null, Collections.emptyMap()).join(); + assertEquals(Collections.nCopies(2, "/" + VALID_PATH), receivedPaths); + + assertThrows(CheckoutArgumentException.class, + () -> transport.invokeSync(operation, DOT_SEGMENT_PATH, authorization, null, null, Collections.emptyMap())); + assertRefusedAsync(() -> transport.invoke(operation, DOT_SEGMENT_PATH, authorization, null, null, Collections.emptyMap()).join()); + assertEquals(2, receivedPaths.size(), "no request may reach the server"); + } + + @Test + void shouldEncodeQueryParametersApartFromThePath() { + localTransport().invokeSync(ClientOperation.QUERY, "event-types", authorization, null, null, + Collections.singletonMap("version", "v2&x=1#y")); + + assertEquals(Collections.singletonList("/event-types"), receivedPaths); + assertEquals(Collections.singletonList("version=v2%26x%3D1%23y"), receivedQueries); + } + + @Test + void shouldSendValidPathsAndRefuseDotSegmentsOnFileUploads() throws IOException { + final ApacheHttpClientTransport transport = localTransport(); + final File file = new File(tempDir, "evidence.pdf"); + Files.write(file.toPath(), "%PDF-1.4".getBytes(StandardCharsets.US_ASCII)); + final FileRequest fileRequest = FileRequest.builder() + .file(file) + .contentType(ContentType.create("application/pdf")) + .purpose(FilePurpose.DISPUTE_EVIDENCE) + .build(); + + transport.submitFileSync("files", authorization, fileRequest); + transport.submitFile("files", authorization, fileRequest).join(); + assertEquals(Collections.nCopies(2, "/files"), receivedPaths); + + assertThrows(CheckoutArgumentException.class, () -> transport.submitFileSync("files/..", authorization, fileRequest)); + assertRefusedAsync(() -> transport.submitFile("files/..", authorization, fileRequest).join()); + assertEquals(2, receivedPaths.size(), "no request may reach the server"); + } + + private static void assertRefusedAsync(final Runnable call) { + final CompletionException exception = assertThrows(CompletionException.class, call::run); + assertInstanceOf(CheckoutArgumentException.class, exception.getCause()); + assertTrue(exception.getCause().getMessage().startsWith("Invalid request path")); } } diff --git a/src/test/java/com/checkout/events/previous/EventsClientImplTest.java b/src/test/java/com/checkout/events/previous/EventsClientImplTest.java index 9721de830..5ffa4d556 100644 --- a/src/test/java/com/checkout/events/previous/EventsClientImplTest.java +++ b/src/test/java/com/checkout/events/previous/EventsClientImplTest.java @@ -59,7 +59,7 @@ void shouldRetrieveAllEventTypes_nullVersion() throws ExecutionException, Interr final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.getAsync("event-types", authorization, EVENT_TYPES_TYPE)) + when(apiClient.queryAsync("event-types", authorization, new EventsClientImpl.EventTypesQuery(null), EVENT_TYPES_TYPE)) .thenReturn(CompletableFuture.completedFuture(response)); final CompletableFuture> eventTypes = eventsClient.retrieveAllEventTypes(null); @@ -72,7 +72,7 @@ void shouldRetrieveAllEventTypes_withVersion() throws ExecutionException, Interr final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.getAsync("event-types?version=v2", authorization, EVENT_TYPES_TYPE)) + when(apiClient.queryAsync("event-types", authorization, new EventsClientImpl.EventTypesQuery("v2"), EVENT_TYPES_TYPE)) .thenReturn(CompletableFuture.completedFuture(response)); final CompletableFuture> eventTypes = eventsClient.retrieveAllEventTypes("v2"); @@ -81,11 +81,11 @@ void shouldRetrieveAllEventTypes_withVersion() throws ExecutionException, Interr } @Test - void shouldRetrieveAllEventTypes_encodesVersion() throws ExecutionException, InterruptedException { + void shouldRetrieveAllEventTypes_withReservedCharactersInVersion() throws ExecutionException, InterruptedException { final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.getAsync("event-types?version=v2%26x%3D1%23y", authorization, EVENT_TYPES_TYPE)) + when(apiClient.queryAsync("event-types", authorization, new EventsClientImpl.EventTypesQuery("v2&x=1#y"), EVENT_TYPES_TYPE)) .thenReturn(CompletableFuture.completedFuture(response)); final CompletableFuture> eventTypes = eventsClient.retrieveAllEventTypes("v2&x=1#y"); @@ -246,7 +246,7 @@ void shouldRetrieveAllEventTypesSync_nullVersion() { final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.get("event-types", authorization, EVENT_TYPES_TYPE)) + when(apiClient.query("event-types", authorization, new EventsClientImpl.EventTypesQuery(null), EVENT_TYPES_TYPE)) .thenReturn(response); final ItemsResponse eventTypes = eventsClient.retrieveAllEventTypesSync(null); @@ -259,7 +259,7 @@ void shouldRetrieveAllEventTypesSync_withVersion() { final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.get("event-types?version=v2", authorization, EVENT_TYPES_TYPE)) + when(apiClient.query("event-types", authorization, new EventsClientImpl.EventTypesQuery("v2"), EVENT_TYPES_TYPE)) .thenReturn(response); final ItemsResponse eventTypes = eventsClient.retrieveAllEventTypesSync("v2"); From d0cc8b49347e2606186e9d60243ecf5ce7e63143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armando=20Rodr=C3=ADguez?= <127134616+armando-rodriguez-cko@users.noreply.github.com> Date: Mon, 9 Nov 2026 22:24:57 +0100 Subject: [PATCH 3/4] fix(client): keep custom ApiClient implementations compiling The Type overloads of query are default methods. A base URI without a path resolves request paths under its root. --- .../checkout/ApacheHttpClientTransport.java | 11 ++++- src/main/java/com/checkout/ApiClient.java | 26 +++++++++- ...acheHttpClientTransportRequestUrlTest.java | 4 ++ .../checkout/ApiClientDefaultQueryTest.java | 48 +++++++++++++++++++ 4 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 src/test/java/com/checkout/ApiClientDefaultQueryTest.java diff --git a/src/main/java/com/checkout/ApacheHttpClientTransport.java b/src/main/java/com/checkout/ApacheHttpClientTransport.java index a06423948..df2cb4cef 100644 --- a/src/main/java/com/checkout/ApacheHttpClientTransport.java +++ b/src/main/java/com/checkout/ApacheHttpClientTransport.java @@ -81,7 +81,7 @@ class ApacheHttpClientTransport implements Transport { final CheckoutConfiguration configuration ) { CheckoutUtils.validateParams("baseUri", baseUri, "httpClientBuilder", httpClientBuilder, "executor", executor); - this.baseUri = baseUri; + this.baseUri = withRootPath(baseUri); this.httpClient = httpClientBuilder .setRedirectStrategy(new CustomAwsRedirectStrategy()) .build(); @@ -405,6 +405,15 @@ String getRequestUrl(final String path) { } } + /** + * A base URI with no path ("https://api.example.com") resolves "payments" to the host + * "api.example.compayments"; the root path "/" keeps the request on the configured host. + */ + private static URI withRootPath(final URI baseUri) { + final String path = baseUri.getRawPath(); + return baseUri.isAbsolute() && !baseUri.isOpaque() && (path == null || path.isEmpty()) ? baseUri.resolve("/") : baseUri; + } + private static void validateRequestPath(final String path) { if (path == null || path.isEmpty() || path.indexOf('?') >= 0 || path.indexOf('#') >= 0) { throw new CheckoutArgumentException(INVALID_REQUEST_PATH); diff --git a/src/main/java/com/checkout/ApiClient.java b/src/main/java/com/checkout/ApiClient.java index 50e3ebc2d..ba9d8086b 100644 --- a/src/main/java/com/checkout/ApiClient.java +++ b/src/main/java/com/checkout/ApiClient.java @@ -45,10 +45,21 @@ public interface ApiClient { * @param authorization the authorization to send * @param filter the object whose fields become the query parameters; null fields are left out * @param responseType the response type, for example a {@code TypeToken} of {@code ItemsResponse} + * The default implementation keeps custom {@code ApiClient} implementations compiling: it delegates to + * {@link #queryAsync(String, SdkAuthorization, Object, Class)} when the type is a plain class, and + * implementations that need generic response types override it. + * * @param the response type * @return the response + * @throws UnsupportedOperationException if the type is generic and the implementation does not override this method */ - CompletableFuture queryAsync(String path, SdkAuthorization authorization, Object filter, Type responseType); + @SuppressWarnings("unchecked") + default CompletableFuture queryAsync(String path, SdkAuthorization authorization, Object filter, Type responseType) { + if (responseType instanceof Class) { + return queryAsync(path, authorization, filter, (Class) responseType); + } + throw new UnsupportedOperationException("queryAsync with a generic response type is not supported by " + getClass().getName()); + } CompletableFuture queryCsvContentAsync(String path, SdkAuthorization authorization, Object filter, String targetFile); @@ -92,10 +103,21 @@ public interface ApiClient { * @param authorization the authorization to send * @param filter the object whose fields become the query parameters; null fields are left out * @param responseType the response type, for example a {@code TypeToken} of {@code ItemsResponse} + * The default implementation keeps custom {@code ApiClient} implementations compiling: it delegates to + * {@link #query(String, SdkAuthorization, Object, Class)} when the type is a plain class, and + * implementations that need generic response types override it. + * * @param the response type * @return the response + * @throws UnsupportedOperationException if the type is generic and the implementation does not override this method */ - T query(String path, SdkAuthorization authorization, Object filter, Type responseType); + @SuppressWarnings("unchecked") + default T query(String path, SdkAuthorization authorization, Object filter, Type responseType) { + if (responseType instanceof Class) { + return query(path, authorization, filter, (Class) responseType); + } + throw new UnsupportedOperationException("query with a generic response type is not supported by " + getClass().getName()); + } ContentResponse queryCsvContent(String path, SdkAuthorization authorization, Object filter, String targetFile); diff --git a/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java b/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java index be2fc52dd..de40d048d 100644 --- a/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java +++ b/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java @@ -98,6 +98,10 @@ void shouldResolveValidRequestPaths() { void shouldResolveUnderCustomBaseUris() { assertEquals("https://proxy.example/checkout/payments/pay_123", transport(URI.create("https://proxy.example/checkout/")).getRequestUrl(VALID_PATH)); + assertEquals("https://proxy.example/payments/pay_123", + transport(URI.create("https://proxy.example")).getRequestUrl(VALID_PATH)); + assertEquals("https://proxy.example:8443/payments/pay_123", + transport(URI.create("https://proxy.example:8443")).getRequestUrl(VALID_PATH)); // Without a trailing slash, URI.resolve replaces the last base segment, as it always has. assertEquals("https://proxy.example/payments/pay_123", transport(URI.create("https://proxy.example/checkout")).getRequestUrl(VALID_PATH)); diff --git a/src/test/java/com/checkout/ApiClientDefaultQueryTest.java b/src/test/java/com/checkout/ApiClientDefaultQueryTest.java new file mode 100644 index 000000000..d423b47ec --- /dev/null +++ b/src/test/java/com/checkout/ApiClientDefaultQueryTest.java @@ -0,0 +1,48 @@ +package com.checkout; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; + +import java.lang.reflect.Type; +import java.util.concurrent.CompletableFuture; + +import org.junit.jupiter.api.Test; + +import com.google.gson.reflect.TypeToken; + +/** + * A custom ApiClient written before the Type overloads of query existed keeps working through the + * default methods. + */ +class ApiClientDefaultQueryTest { + + private static final Type GENERIC_TYPE = new TypeToken>() { + }.getType(); + + private final ApiClient customClient = mock(ApiClient.class, withSettings().defaultAnswer(CALLS_REAL_METHODS)); + private final SdkAuthorization authorization = new SdkAuthorization(PlatformType.DEFAULT, "sk_test"); + + @Test + void shouldDelegatePlainClassTypesToTheClassOverloads() { + final EmptyResponse response = new EmptyResponse(); + final CompletableFuture future = CompletableFuture.completedFuture(response); + when(customClient.query(eq("x"), eq(authorization), any(), eq(EmptyResponse.class))).thenReturn(response); + when(customClient.queryAsync(eq("x"), eq(authorization), any(), eq(EmptyResponse.class))).thenReturn(future); + + assertSame(response, customClient.query("x", authorization, new Object(), (Type) EmptyResponse.class)); + assertSame(future, customClient.queryAsync("x", authorization, new Object(), (Type) EmptyResponse.class)); + } + + @Test + void shouldReportGenericTypesThatTheImplementationDoesNotSupport() { + assertThrows(UnsupportedOperationException.class, () -> customClient.query("x", authorization, new Object(), GENERIC_TYPE)); + assertThrows(UnsupportedOperationException.class, () -> customClient.queryAsync("x", authorization, new Object(), GENERIC_TYPE)); + } + +} From 9a5c3bc34267923e7c5c9a40b9269da608bd5111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armando=20Rodr=C3=ADguez?= <127134616+armando-rodriguez-cko@users.noreply.github.com> Date: Mon, 9 Nov 2026 23:06:40 +0100 Subject: [PATCH 4/4] fix(client): keep the event types request shape The event types version is sent as an encoded query on the same call as before, so no ApiClient overloads are added. The transport accepts only an already encoded query after the path. --- .../checkout/ApacheHttpClientTransport.java | 19 +++++--- src/main/java/com/checkout/ApiClient.java | 46 ------------------ src/main/java/com/checkout/ApiClientImpl.java | 16 ------- .../events/previous/EventsClientImpl.java | 23 +++++---- ...acheHttpClientTransportRequestUrlTest.java | 8 +++- .../checkout/ApiClientDefaultQueryTest.java | 48 ------------------- .../events/previous/EventsClientImplTest.java | 10 ++-- 7 files changed, 40 insertions(+), 130 deletions(-) delete mode 100644 src/test/java/com/checkout/ApiClientDefaultQueryTest.java diff --git a/src/main/java/com/checkout/ApacheHttpClientTransport.java b/src/main/java/com/checkout/ApacheHttpClientTransport.java index df2cb4cef..774cf9b10 100644 --- a/src/main/java/com/checkout/ApacheHttpClientTransport.java +++ b/src/main/java/com/checkout/ApacheHttpClientTransport.java @@ -41,6 +41,7 @@ import java.util.concurrent.Executor; import java.util.stream.Collectors; import java.util.function.Supplier; +import java.util.regex.Pattern; import io.github.resilience4j.circuitbreaker.CircuitBreaker; import io.github.resilience4j.ratelimiter.RateLimiter; import io.github.resilience4j.retry.Retry; @@ -63,6 +64,7 @@ class ApacheHttpClientTransport implements Transport { private static final String PURPOSE = "purpose"; private static final String PATH = "path"; private static final String INVALID_REQUEST_PATH = "Invalid request path"; + private static final Pattern ENCODED_QUERY = Pattern.compile("[A-Za-z0-9._~*+%=&-]+"); private final URI baseUri; private final CloseableHttpClient httpClient; @@ -384,9 +386,10 @@ private HttpUriRequest buildRequest(final ClientOperation clientOperation, final /** * Resolves a request path against the base URI. The path must be relative and made of non-empty - * segments, none of them "." or ".." (also when percent-encoded or followed by ";"), with no "?" or - * "#": query parameters are added separately. The resolved URL must keep the scheme, host, port and - * base path of the base URI. + * segments, none of them "." or ".." (also when percent-encoded or followed by ";"). It may end with + * an already encoded query ("?name=value&..." made only of unreserved characters, "%", "+", "=" + * and "&"), and never with a "#". The resolved URL must keep the scheme, host, port and base path + * of the base URI. * * @param path the request path, as built by {@code AbstractClient.buildPath} * @return the absolute request URL @@ -415,10 +418,15 @@ private static URI withRootPath(final URI baseUri) { } private static void validateRequestPath(final String path) { - if (path == null || path.isEmpty() || path.indexOf('?') >= 0 || path.indexOf('#') >= 0) { + if (path == null || path.isEmpty() || path.indexOf('#') >= 0) { throw new CheckoutArgumentException(INVALID_REQUEST_PATH); } - for (final String segment : path.split("/", -1)) { + final int queryStart = path.indexOf('?'); + if (queryStart >= 0 && !ENCODED_QUERY.matcher(path.substring(queryStart + 1)).matches()) { + throw new CheckoutArgumentException(INVALID_REQUEST_PATH); + } + final String pathOnly = queryStart >= 0 ? path.substring(0, queryStart) : path; + for (final String segment : pathOnly.split("/", -1)) { if (isEmptyOrDotSegment(segment)) { throw new CheckoutArgumentException(INVALID_REQUEST_PATH); } @@ -444,7 +452,6 @@ private boolean isUnderBaseUri(final URI resolved) { && baseUri.getPort() == resolved.getPort() && resolved.getRawPath() != null && resolved.getRawPath().startsWith(baseDirectory) - && resolved.getRawQuery() == null && resolved.getRawFragment() == null; } } diff --git a/src/main/java/com/checkout/ApiClient.java b/src/main/java/com/checkout/ApiClient.java index ba9d8086b..5e50a7e7b 100644 --- a/src/main/java/com/checkout/ApiClient.java +++ b/src/main/java/com/checkout/ApiClient.java @@ -38,29 +38,6 @@ public interface ApiClient { CompletableFuture queryAsync(String path, SdkAuthorization authorization, Object filter, Class responseType); - /** - * Sends a GET request with the filter's fields as query parameters, for a generic response type. - * - * @param path the request path, relative to the base URI - * @param authorization the authorization to send - * @param filter the object whose fields become the query parameters; null fields are left out - * @param responseType the response type, for example a {@code TypeToken} of {@code ItemsResponse} - * The default implementation keeps custom {@code ApiClient} implementations compiling: it delegates to - * {@link #queryAsync(String, SdkAuthorization, Object, Class)} when the type is a plain class, and - * implementations that need generic response types override it. - * - * @param the response type - * @return the response - * @throws UnsupportedOperationException if the type is generic and the implementation does not override this method - */ - @SuppressWarnings("unchecked") - default CompletableFuture queryAsync(String path, SdkAuthorization authorization, Object filter, Type responseType) { - if (responseType instanceof Class) { - return queryAsync(path, authorization, filter, (Class) responseType); - } - throw new UnsupportedOperationException("queryAsync with a generic response type is not supported by " + getClass().getName()); - } - CompletableFuture queryCsvContentAsync(String path, SdkAuthorization authorization, Object filter, String targetFile); CompletableFuture submitFileAsync(String path, SdkAuthorization authorization, AbstractFileRequest request, Class responseType); @@ -96,29 +73,6 @@ default CompletableFuture queryAsync(String path, Sd T query(String path, SdkAuthorization authorization, Object filter, Class responseType); - /** - * Synchronous version of {@link #queryAsync(String, SdkAuthorization, Object, Type)}. - * - * @param path the request path, relative to the base URI - * @param authorization the authorization to send - * @param filter the object whose fields become the query parameters; null fields are left out - * @param responseType the response type, for example a {@code TypeToken} of {@code ItemsResponse} - * The default implementation keeps custom {@code ApiClient} implementations compiling: it delegates to - * {@link #query(String, SdkAuthorization, Object, Class)} when the type is a plain class, and - * implementations that need generic response types override it. - * - * @param the response type - * @return the response - * @throws UnsupportedOperationException if the type is generic and the implementation does not override this method - */ - @SuppressWarnings("unchecked") - default T query(String path, SdkAuthorization authorization, Object filter, Type responseType) { - if (responseType instanceof Class) { - return query(path, authorization, filter, (Class) responseType); - } - throw new UnsupportedOperationException("query with a generic response type is not supported by " + getClass().getName()); - } - ContentResponse queryCsvContent(String path, SdkAuthorization authorization, Object filter, String targetFile); T submitFile(String path, SdkAuthorization authorization, AbstractFileRequest request, Class responseType); diff --git a/src/main/java/com/checkout/ApiClientImpl.java b/src/main/java/com/checkout/ApiClientImpl.java index 847477dc0..0478c1952 100644 --- a/src/main/java/com/checkout/ApiClientImpl.java +++ b/src/main/java/com/checkout/ApiClientImpl.java @@ -180,14 +180,6 @@ public CompletableFuture queryAsync(final String pat final SdkAuthorization authorization, final Object filter, final Class responseType) { - return queryAsync(path, authorization, filter, (Type) responseType); - } - - @Override - public CompletableFuture queryAsync(final String path, - final SdkAuthorization authorization, - final Object filter, - final Type responseType) { validateParams(PATH, path, AUTHORIZATION, authorization, "filter", filter); return executeAsyncOrSync( () -> query(path, authorization, filter, responseType), @@ -444,14 +436,6 @@ public T query(final String path, final SdkAuthorization authorization, final Object filter, final Class responseType) { - return query(path, authorization, filter, (Type) responseType); - } - - @Override - public T query(final String path, - final SdkAuthorization authorization, - final Object filter, - final Type responseType) { validateParams(PATH, path, AUTHORIZATION, authorization, "filter", filter); final Map params = serializer.fromJson(serializer.toJson(filter), new TypeToken>() { diff --git a/src/main/java/com/checkout/events/previous/EventsClientImpl.java b/src/main/java/com/checkout/events/previous/EventsClientImpl.java index d91e88f65..6f2d7f9a1 100644 --- a/src/main/java/com/checkout/events/previous/EventsClientImpl.java +++ b/src/main/java/com/checkout/events/previous/EventsClientImpl.java @@ -2,7 +2,10 @@ import static com.checkout.common.CheckoutUtils.validateParams; +import java.io.UnsupportedEncodingException; import java.lang.reflect.Type; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.concurrent.CompletableFuture; import com.checkout.AbstractClient; @@ -13,8 +16,6 @@ import com.checkout.SdkAuthorizationType; import com.google.gson.reflect.TypeToken; -import lombok.Value; - public class EventsClientImpl extends AbstractClient implements EventsClient { private static final String EVENT_TYPES_PATH = "event-types"; @@ -32,7 +33,7 @@ public EventsClientImpl(final ApiClient apiClient, final CheckoutConfiguration c @Override public CompletableFuture> retrieveAllEventTypes(final String version) { - return apiClient.queryAsync(EVENT_TYPES_PATH, sdkAuthorization(), new EventTypesQuery(version), EVENT_TYPES_TYPE); + return apiClient.getAsync(eventTypesPath(version), sdkAuthorization(), EVENT_TYPES_TYPE); } @Override @@ -68,7 +69,7 @@ public CompletableFuture retryAllWebhooks(final String eventId) { // Synchronous methods @Override public ItemsResponse retrieveAllEventTypesSync(final String version) { - return apiClient.query(EVENT_TYPES_PATH, sdkAuthorization(), new EventTypesQuery(version), EVENT_TYPES_TYPE); + return apiClient.get(eventTypesPath(version), sdkAuthorization(), EVENT_TYPES_TYPE); } @Override @@ -103,11 +104,17 @@ public EmptyResponse retryAllWebhooksSync(final String eventId) { // Common methods /** - * Query of the event types request: {@code version} is sent as a query parameter, and left out when null. + * The event types path, with {@code version} as a form-encoded query parameter when it is set. */ - @Value - static class EventTypesQuery { - String version; + private static String eventTypesPath(final String version) { + if (version == null) { + return EVENT_TYPES_PATH; + } + try { + return EVENT_TYPES_PATH + "?version=" + URLEncoder.encode(version, StandardCharsets.UTF_8.name()); + } catch (final UnsupportedEncodingException e) { + throw new IllegalStateException("UTF-8 is always supported", e); + } } protected void validateRetrieveEventsRequest(final RetrieveEventsRequest retrieveEventsRequest) { diff --git a/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java b/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java index de40d048d..6d4ffcf5b 100644 --- a/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java +++ b/src/test/java/com/checkout/ApacheHttpClientTransportRequestUrlTest.java @@ -92,6 +92,8 @@ void shouldResolveValidRequestPaths() { assertEquals("https://api.sandbox.checkout.com/payments/pay_1%2Frefunds", transport.getRequestUrl("payments/pay_1%2Frefunds")); assertEquals("https://api.sandbox.checkout.com/payments/...", transport.getRequestUrl("payments/...")); assertEquals("https://api.sandbox.checkout.com/ideal-external/issuers", transport.getRequestUrl("ideal-external/issuers")); + assertEquals("https://api.sandbox.checkout.com/event-types?version=v2%26x%3D1%23y", + transport.getRequestUrl("event-types?version=v2%26x%3D1%23y")); } @Test @@ -122,7 +124,11 @@ void shouldResolveUnderCustomBaseUris() { "/payments/pay_1", "//example.com/payments", "payments/pay_1/refunds?", - "event-types?version=v2", + "event-types?version=v2#x", + "event-types?version=../x", + "event-types?version=a?b", + "a?b/../c", + "payments/../x?version=v2", "payments/pay_1#refunds", "https:evil.example/payments", "https://evil.example/payments", diff --git a/src/test/java/com/checkout/ApiClientDefaultQueryTest.java b/src/test/java/com/checkout/ApiClientDefaultQueryTest.java deleted file mode 100644 index d423b47ec..000000000 --- a/src/test/java/com/checkout/ApiClientDefaultQueryTest.java +++ /dev/null @@ -1,48 +0,0 @@ -package com.checkout; - -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.CALLS_REAL_METHODS; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.mockito.Mockito.withSettings; - -import java.lang.reflect.Type; -import java.util.concurrent.CompletableFuture; - -import org.junit.jupiter.api.Test; - -import com.google.gson.reflect.TypeToken; - -/** - * A custom ApiClient written before the Type overloads of query existed keeps working through the - * default methods. - */ -class ApiClientDefaultQueryTest { - - private static final Type GENERIC_TYPE = new TypeToken>() { - }.getType(); - - private final ApiClient customClient = mock(ApiClient.class, withSettings().defaultAnswer(CALLS_REAL_METHODS)); - private final SdkAuthorization authorization = new SdkAuthorization(PlatformType.DEFAULT, "sk_test"); - - @Test - void shouldDelegatePlainClassTypesToTheClassOverloads() { - final EmptyResponse response = new EmptyResponse(); - final CompletableFuture future = CompletableFuture.completedFuture(response); - when(customClient.query(eq("x"), eq(authorization), any(), eq(EmptyResponse.class))).thenReturn(response); - when(customClient.queryAsync(eq("x"), eq(authorization), any(), eq(EmptyResponse.class))).thenReturn(future); - - assertSame(response, customClient.query("x", authorization, new Object(), (Type) EmptyResponse.class)); - assertSame(future, customClient.queryAsync("x", authorization, new Object(), (Type) EmptyResponse.class)); - } - - @Test - void shouldReportGenericTypesThatTheImplementationDoesNotSupport() { - assertThrows(UnsupportedOperationException.class, () -> customClient.query("x", authorization, new Object(), GENERIC_TYPE)); - assertThrows(UnsupportedOperationException.class, () -> customClient.queryAsync("x", authorization, new Object(), GENERIC_TYPE)); - } - -} diff --git a/src/test/java/com/checkout/events/previous/EventsClientImplTest.java b/src/test/java/com/checkout/events/previous/EventsClientImplTest.java index 5ffa4d556..6796a9a48 100644 --- a/src/test/java/com/checkout/events/previous/EventsClientImplTest.java +++ b/src/test/java/com/checkout/events/previous/EventsClientImplTest.java @@ -59,7 +59,7 @@ void shouldRetrieveAllEventTypes_nullVersion() throws ExecutionException, Interr final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.queryAsync("event-types", authorization, new EventsClientImpl.EventTypesQuery(null), EVENT_TYPES_TYPE)) + when(apiClient.getAsync("event-types", authorization, EVENT_TYPES_TYPE)) .thenReturn(CompletableFuture.completedFuture(response)); final CompletableFuture> eventTypes = eventsClient.retrieveAllEventTypes(null); @@ -72,7 +72,7 @@ void shouldRetrieveAllEventTypes_withVersion() throws ExecutionException, Interr final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.queryAsync("event-types", authorization, new EventsClientImpl.EventTypesQuery("v2"), EVENT_TYPES_TYPE)) + when(apiClient.getAsync("event-types?version=v2", authorization, EVENT_TYPES_TYPE)) .thenReturn(CompletableFuture.completedFuture(response)); final CompletableFuture> eventTypes = eventsClient.retrieveAllEventTypes("v2"); @@ -85,7 +85,7 @@ void shouldRetrieveAllEventTypes_withReservedCharactersInVersion() throws Execut final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.queryAsync("event-types", authorization, new EventsClientImpl.EventTypesQuery("v2&x=1#y"), EVENT_TYPES_TYPE)) + when(apiClient.getAsync("event-types?version=v2%26x%3D1%23y", authorization, EVENT_TYPES_TYPE)) .thenReturn(CompletableFuture.completedFuture(response)); final CompletableFuture> eventTypes = eventsClient.retrieveAllEventTypes("v2&x=1#y"); @@ -246,7 +246,7 @@ void shouldRetrieveAllEventTypesSync_nullVersion() { final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.query("event-types", authorization, new EventsClientImpl.EventTypesQuery(null), EVENT_TYPES_TYPE)) + when(apiClient.get("event-types", authorization, EVENT_TYPES_TYPE)) .thenReturn(response); final ItemsResponse eventTypes = eventsClient.retrieveAllEventTypesSync(null); @@ -259,7 +259,7 @@ void shouldRetrieveAllEventTypesSync_withVersion() { final ItemsResponse response = createMockItemsResponse(); createMockSdkCredentials(); - when(apiClient.query("event-types", authorization, new EventsClientImpl.EventTypesQuery("v2"), EVENT_TYPES_TYPE)) + when(apiClient.get("event-types?version=v2", authorization, EVENT_TYPES_TYPE)) .thenReturn(response); final ItemsResponse eventTypes = eventsClient.retrieveAllEventTypesSync("v2");