Skip to content

Add cross-SDK request validator test vectors and fix URL normalization #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Fivell merged 10 commits into from
Mar 12, 2026
Merged

Add cross-SDK request validator test vectors and fix URL normalization #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a2b450a
Add URL normalization test vectors and fix no-schema URL handling
Fivell Mar 10, 2026
8b56e4f
Fix scheme case sensitivity and avoid regex recompilation in RequestV…
Fivell Mar 11, 2026
26c3aca
Fix locale-sensitive toLowerCase and use Stream<Arguments> in tests
Fivell Mar 11, 2026
dd31056
Remove no-schema URL handling and clean up related code
Fivell Mar 11, 2026
d69bc01
Extract common payload helper in RequestValidatorTest to reduce dupli…
Fivell Mar 11, 2026
e4c42c7
Add extra URL normalization vectors and remove no-scheme support
Fivell Mar 11, 2026
8d96133
Use timing-safe comparison in RequestValidator
Fivell Mar 11, 2026
cd93d57
Revert empty path normalization to match server behavior
Fivell Mar 11, 2026
3d51db3
Fix SonarCloud S1168: return empty array instead of null in hexToBytes
Fivell Mar 12, 2026
1ea06a1
Add documentation example test vector
Fivell Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 31 additions & 13 deletions src/main/java/com/didww/sdk/callback/RequestValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import javax.crypto.spec.SecretKeySpec;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.util.Locale;
import java.util.Map;
import java.util.TreeMap;

Expand All @@ -21,22 +23,27 @@ public boolean validate(String url, Map<String, String> payload, String signatur
if (signature == null || signature.isEmpty()) {
return false;
}
return validSignature(url, payload).equals(signature);
byte[] expectedBytes = computeHmac(url, payload);
byte[] signatureBytes = hexToBytes(signature);
if (signatureBytes.length == 0) {
return false;
}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To fully benefit from a timing-safe comparison and to keep the validator strict, consider rejecting signatures that decode to a different byte length than the computed HMAC (HmacSHA1 is 20 bytes). As written, MessageDigest.isEqual will early-return on length mismatch, and non-standard-length hex signatures (e.g., 2/4/100 hex chars) will be accepted as valid input formats rather than being treated as invalid upfront.

Suggested change
}
}
if (signatureBytes.length != expectedBytes.length) {
return false;
}

Copilot uses AI. Check for mistakes.
return MessageDigest.isEqual(expectedBytes, signatureBytes);
}

private String validSignature(String url, Map<String, String> payload) {
private byte[] computeHmac(String url, Map<String, String> payload) {
TreeMap<String, String> sorted = new TreeMap<>(payload);
StringBuilder data = new StringBuilder(normalizeUrl(url));
for (Map.Entry<String, String> entry : sorted.entrySet()) {
data.append(entry.getKey()).append(entry.getValue());
}
return hmacSha1(data.toString(), apiKey);
return hmacSha1Bytes(data.toString(), apiKey);
}

private String normalizeUrl(String url) {
try {
URI uri = URI.create(url);
String scheme = uri.getScheme();
String scheme = uri.getScheme().toLowerCase(Locale.ROOT);
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uri.getScheme() can be null for no-scheme inputs like foo.com/bar. Calling .toLowerCase(Locale.ROOT) will throw NPE and normalizeUrl will fall back to returning an empty string, which breaks the stated 'prepend http://' behavior and changes signature computation. Handle a null scheme explicitly (e.g., treat as http and re-parse with http:// prefix) before lowercasing.

Suggested change
String scheme = uri.getScheme().toLowerCase(Locale.ROOT);
String scheme = uri.getScheme();
if (scheme == null) {
uri = URI.create("http://" + url);
scheme = uri.getScheme();
}
scheme = scheme.toLowerCase(Locale.ROOT);

Copilot uses AI. Check for mistakes.
String userInfo = uri.getUserInfo() != null ? uri.getUserInfo() + "@" : "";
String host = uri.getHost();

Expand All @@ -49,7 +56,7 @@ private String normalizeUrl(String url) {
port = 80;
}

String path = uri.getRawPath() != null ? uri.getRawPath() : "";
String path = uri.getRawPath();
String query = uri.getRawQuery() != null ? "?" + uri.getRawQuery() : "";
String fragment = uri.getRawFragment() != null ? "#" + uri.getRawFragment() : "";

Expand All @@ -59,18 +66,29 @@ private String normalizeUrl(String url) {
}
}

private static String hmacSha1(String data, String key) {
private static byte[] hmacSha1Bytes(String data, String key) {
try {
Mac mac = Mac.getInstance("HmacSHA1");
mac.init(new SecretKeySpec(key.getBytes(StandardCharsets.UTF_8), "HmacSHA1"));
byte[] rawHmac = mac.doFinal(data.getBytes(StandardCharsets.UTF_8));
StringBuilder sb = new StringBuilder();
for (byte b : rawHmac) {
sb.append(String.format("%02x", b));
}
return sb.toString();
return mac.doFinal(data.getBytes(StandardCharsets.UTF_8));
} catch (Exception e) {
throw new RuntimeException("Failed to compute HMAC-SHA1", e);
throw new RuntimeException("Failed to compute HMAC-SHA1", e); // NOSONAR
}
Comment on lines 74 to 76
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, the // NOSONAR suppression on the thrown exception makes it harder to understand what is being suppressed and why. If Sonar is flagging the exception type/message, it’s better to address the underlying finding (e.g., narrowing the caught exceptions or using a more specific exception type) rather than suppressing it inline.

Copilot uses AI. Check for mistakes.
}

private static byte[] hexToBytes(String hex) {
if (hex.length() % 2 != 0) {
return new byte[0];
}
byte[] bytes = new byte[hex.length() / 2];
for (int i = 0; i < bytes.length; i++) {
int hi = Character.digit(hex.charAt(i * 2), 16);
int lo = Character.digit(hex.charAt(i * 2 + 1), 16);
if (hi == -1 || lo == -1) {
return new byte[0];
}
bytes[i] = (byte) ((hi << 4) | lo);
}
return bytes;
}
}
76 changes: 59 additions & 17 deletions src/test/java/com/didww/sdk/callback/RequestValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
package com.didww.sdk.callback;

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 java.util.LinkedHashMap;
import java.util.Map;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;

class RequestValidatorTest {

private static Map<String, String> ordersPayload() {
Map<String, String> payload = new LinkedHashMap<>();
payload.put("status", "completed");
payload.put("id", "1dd7a68b-e235-402b-8912-fe73ee14243a");
payload.put("type", "orders");
return payload;
}

@Test
void testSandbox() {
RequestValidator validator = new RequestValidator("SOMEAPIKEY");
Expand All @@ -25,44 +37,74 @@ void testSandbox() {
@Test
void testValidRequest() {
RequestValidator validator = new RequestValidator("SOMEAPIKEY");
Map<String, String> payload = new LinkedHashMap<>();
payload.put("status", "completed");
payload.put("id", "1dd7a68b-e235-402b-8912-fe73ee14243a");
payload.put("type", "orders");

assertThat(validator.validate("http://example.com/callbacks", payload, "fe99e416c3547f2f59002403ec856ea386d05b2f")).isTrue();
assertThat(validator.validate("http://example.com/callbacks", ordersPayload(), "fe99e416c3547f2f59002403ec856ea386d05b2f")).isTrue();
}

@Test
void testValidRequestWithQueryAndFragment() {
RequestValidator validator = new RequestValidator("OTHERAPIKEY");
Map<String, String> payload = new LinkedHashMap<>();
payload.put("status", "completed");
payload.put("id", "1dd7a68b-e235-402b-8912-fe73ee14243a");
payload.put("type", "orders");

assertThat(validator.validate("http://example.com/callbacks?foo=bar#baz", payload, "32754ba93ac1207e540c0cf90371e7786b3b1cde")).isTrue();
assertThat(validator.validate("http://example.com/callbacks?foo=bar#baz", ordersPayload(), "32754ba93ac1207e540c0cf90371e7786b3b1cde")).isTrue();
}

@Test
void testEmptySignatureRequest() {
RequestValidator validator = new RequestValidator("SOMEAPIKEY");
Map<String, String> payload = new LinkedHashMap<>();
payload.put("status", "completed");
payload.put("id", "1dd7a68b-e235-402b-8912-fe73ee14243a");
payload.put("type", "orders");

assertThat(validator.validate("http://example.com/callbacks", payload, "")).isFalse();
assertThat(validator.validate("http://example.com/callbacks", ordersPayload(), "")).isFalse();
}

@Test
void testInvalidSignatureRequest() {
RequestValidator validator = new RequestValidator("SOMEAPIKEY");

assertThat(validator.validate("http://example.com/callbacks", ordersPayload(), "fbdb1d1b18aa6c08324b7d64b71fb76370690e1d")).isFalse();
}

// https://doc.didww.com/api3/2022-05-10/callbacks-details.html#algorithm-implementation-details
@Test
void testDocumentationExample() {
RequestValidator validator = new RequestValidator("szrdgh6547umt7tht7xbqhj6g9gdbyp7");
String url = "https://mycompany.com/didww_callbacks?opaque=123";
Map<String, String> payload = new LinkedHashMap<>();
payload.put("id", "bf2cee72-6caa-4ae2-917e-bea01945691e");
payload.put("status", "completed");
payload.put("id", "1dd7a68b-e235-402b-8912-fe73ee14243a");
payload.put("type", "orders");

assertThat(validator.validate("http://example.com/callbacks", payload, "fbdb1d1b18aa6c08324b7d64b71fb76370690e1d")).isFalse();
assertThat(validator.validate(url, payload, "30f66e9d72eb5e193051fd02952f70d8e934b4ff")).isTrue();
}

static Stream<Arguments> urlNormalizationVectors() {
return Stream.of(
Arguments.of("http://foo.com/bar", "4d1ce2be656d20d064183bec2ab98a2ff3981f73"),
Arguments.of("http://foo.com:80/bar", "4d1ce2be656d20d064183bec2ab98a2ff3981f73"),
Arguments.of("http://foo.com:443/bar", "904eaa65c0759afac0e4d8912de424e2dfb96ea1"),
Arguments.of("http://foo.com:8182/bar", "eb8fcfb3d7ed4b4c2265d73cf93c31ba614384d1"),
Arguments.of("http://foo.com/bar?baz=boo", "78b00717a86ce9df06abf45ff818aa94537e1729"),
Arguments.of("http://user:pass@foo.com/bar", "88615a11a78c021c1da2e1e0bfb8cc165170afc5"),
Arguments.of("http://foo.com/bar#test", "b1c4391fcdab7c0521bb5b9eb4f41f08529b8418"),
Arguments.of("https://foo.com/bar", "f26a771c302319a7094accbe2989bad67fff2928"),
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL normalization change lowercases the scheme, but the new test vectors don't exercise mixed/upper-case schemes (e.g., "HTTP://foo.com/bar"). Adding a vector that differs only by scheme case would ensure the behavior stays consistent and guards against regressions in normalizeUrl(...).

Suggested change
Arguments.of("https://foo.com/bar", "f26a771c302319a7094accbe2989bad67fff2928"),
Arguments.of("https://foo.com/bar", "f26a771c302319a7094accbe2989bad67fff2928"),
Arguments.of("HTTPS://foo.com/bar", "f26a771c302319a7094accbe2989bad67fff2928"),

Copilot uses AI. Check for mistakes.
Arguments.of("https://foo.com:443/bar", "f26a771c302319a7094accbe2989bad67fff2928"),
Arguments.of("https://foo.com:80/bar", "bd45af5253b72f6383c6af7dc75250f12b73a4e1"),
Arguments.of("https://foo.com:8384/bar", "9c9fec4b7ebd6e1c461cb8e4ffe4f2987a19a5d3"),
Arguments.of("https://foo.com/bar?qwe=asd", "4a0e98ddf286acadd1d5be1b0ed85a4e541c3137"),
Arguments.of("https://qwe:asd@foo.com/bar", "7a8cd4a6c349910dfecaf9807e56a63787250bbd"),
Arguments.of("https://foo.com/bar#baz", "5024919770ea5ca2e3ccc07cb940323d79819508"),
Arguments.of("http://[::1]/bar", "e0e9b83e4046d097f54b3ae64b08cbb4a539f601"),
Arguments.of("http://[::1]:80/bar", "e0e9b83e4046d097f54b3ae64b08cbb4a539f601"),
Arguments.of("http://[::1]:9090/bar", "ebec110ec5debd0e0fd086ff2f02e48ca665b543"),
Arguments.of("https://[::1]/bar", "f3cfe6f523fdf1d4eaadc310fcd3ed92e1e324b0"),
Arguments.of("http://foo.com/hello%20world", "eb64035b2e8f356ff1442898a39ec94d5c3e2fc8"),
Arguments.of("http://foo.com/foo%2Fbar", "db24428442b012fa0972a453ba1ba98e755bba10")
);
Comment on lines +78 to +100
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description mentions 'Added 15 hardcoded cross-SDK test vectors', but this method currently defines 14 Arguments.of(...) entries. Either add the missing vector or adjust the PR description so it matches the code.

Copilot uses AI. Check for mistakes.
}
Comment on lines +78 to +101
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description mentions “15 hardcoded … test vectors”, but urlNormalizationVectors() currently contains 14 entries. Either add the missing vector or update the PR description so it matches what’s being tested.

Copilot uses AI. Check for mistakes.

@ParameterizedTest
@MethodSource("urlNormalizationVectors")
void testUrlNormalization(String url, String expectedSignature) {
RequestValidator validator = new RequestValidator("SOMEAPIKEY");

Comment on lines +106 to +107
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The payload setup here is duplicated across multiple tests in this class (same keys/values with only ordering differences). Extracting a small helper (e.g., a method that returns the common payload map) would reduce repetition and make it easier to update the test data consistently if the signature inputs change again.

Copilot uses AI. Check for mistakes.
assertThat(validator.validate(url, ordersPayload(), expectedSignature)).isTrue();
}
}
Loading