From ceab6426263e0fe7c4d413c9f6bd1fab9b55d65c Mon Sep 17 00:00:00 2001 From: Kai Hudalla Date: Thu, 25 Sep 2025 08:20:12 +0200 Subject: [PATCH] Move validation of authority to UriValidator The code for validating the authority component of a uProtocol URI has been moved from the UriSerializer class to the UriValidator class. This change centralizes the validation logic, making it easier to maintain and ensuring consistent validation across different parts of the codebase. --- .../uri/serializer/UriSerializer.java | 32 +----- .../uprotocol/uri/validator/UriValidator.java | 73 ++++++++++--- .../uri/serializer/UriSerializerTest.java | 7 +- .../uri/validator/UriValidatorTest.java | 102 +++++++++++++----- .../features/uuri_uri_serialization.feature | 1 + 5 files changed, 144 insertions(+), 71 deletions(-) diff --git a/src/main/java/org/eclipse/uprotocol/uri/serializer/UriSerializer.java b/src/main/java/org/eclipse/uprotocol/uri/serializer/UriSerializer.java index eea897c..2784c4f 100644 --- a/src/main/java/org/eclipse/uprotocol/uri/serializer/UriSerializer.java +++ b/src/main/java/org/eclipse/uprotocol/uri/serializer/UriSerializer.java @@ -13,10 +13,8 @@ package org.eclipse.uprotocol.uri.serializer; import java.net.URI; -import java.net.URISyntaxException; import java.util.Objects; import java.util.Optional; -import java.util.regex.Pattern; import org.eclipse.uprotocol.uri.validator.UriValidator; import org.eclipse.uprotocol.v1.UUri; @@ -35,8 +33,6 @@ public final class UriSerializer { */ public static final String SCHEME_UP = "up"; - private static final Pattern AUTHORITY_PATTERN = Pattern.compile("^[a-z0-9-._~]{0,128}$"); - private UriSerializer() { // prevent instantiation } @@ -105,12 +101,10 @@ public static String serialize(UUri uuri, boolean includeScheme) { * @throws NullPointerException if the URI is null. * @throws IllegalArgumentException if the URI is invalid. */ - // [impl->dsn~uri-authority-name-length~1] // [impl->dsn~uri-scheme~1] - // [impl->dsn~uri-host-only~2] - // [impl->dsn~uri-authority-mapping~1] // [impl->dsn~uri-path-mapping~1] // [impl->req~uri-serialization~1] + // [impl->dsn~uri-authority-mapping~1] public static UUri deserialize(String uProtocolUri) { Objects.requireNonNull(uProtocolUri); final var parsedUri = URI.create(uProtocolUri); @@ -125,12 +119,10 @@ public static UUri deserialize(String uProtocolUri) { * @throws NullPointerException if the URI is null. * @throws IllegalArgumentException if the URI is invalid. */ - // [impl->dsn~uri-authority-name-length~1] // [impl->dsn~uri-scheme~1] - // [impl->dsn~uri-host-only~2] - // [impl->dsn~uri-authority-mapping~1] // [impl->dsn~uri-path-mapping~1] // [impl->req~uri-serialization~1] + // [impl->dsn~uri-authority-mapping~1] public static UUri deserialize(URI uProtocolUri) { Objects.requireNonNull(uProtocolUri); @@ -143,25 +135,7 @@ public static UUri deserialize(URI uProtocolUri) { if (uProtocolUri.getFragment() != null) { throw new IllegalArgumentException("uProtocol URI must not contain fragment"); } - - String authority; - try { - // this should work if authority is server-based (i.e. contains a host) - var uriWithServerAuthority = uProtocolUri.parseServerAuthority(); - // we can then verify that the authority does neither contain user info nor port - UriValidator.validateParsedAuthority(uriWithServerAuthority); - authority = uriWithServerAuthority.getHost(); - } catch (URISyntaxException e) { - // the authority is not server-based but might still be valid according to the UUri spec, - // we just need to make sure that it either is the wildcard authority or contains allowed - // characters only - authority = uProtocolUri.getAuthority(); - if (authority != null && !"*".equals(authority) && !AUTHORITY_PATTERN.matcher(authority).matches()) { - throw new IllegalArgumentException( - "uProtocol URI authority contains invalid characters", - e); - } - } + String authority = UriValidator.validateAuthority(uProtocolUri); final var pathSegments = uProtocolUri.getPath().split("/"); if (pathSegments.length != 4) { diff --git a/src/main/java/org/eclipse/uprotocol/uri/validator/UriValidator.java b/src/main/java/org/eclipse/uprotocol/uri/validator/UriValidator.java index 5f0af76..58f914f 100644 --- a/src/main/java/org/eclipse/uprotocol/uri/validator/UriValidator.java +++ b/src/main/java/org/eclipse/uprotocol/uri/validator/UriValidator.java @@ -16,6 +16,7 @@ import java.net.URISyntaxException; import java.util.Objects; import java.util.Optional; +import java.util.regex.Pattern; import org.eclipse.uprotocol.communication.UStatusException; import org.eclipse.uprotocol.transport.UTransport; @@ -28,6 +29,13 @@ */ public final class UriValidator { + /** + * The maximum length of an authority name. + */ + public static final int AUTHORITY_NAME_MAX_LENGTH = 128; + + private static final Pattern AUTHORITY_PATTERN = Pattern.compile("^[a-z0-9-._~]*$"); + private UriValidator() { // prevent instantiation } @@ -57,7 +65,7 @@ public static void validate(UUri uuri) { .ifPresent(name -> { try { var uri = new URI(null, name, null, null, null); - validateParsedAuthority(uri); + validateAuthority(uri); } catch (URISyntaxException e) { throw new IllegalArgumentException("Invalid authority name", e); } @@ -84,21 +92,50 @@ public static void validateResourceId(int resourceId) { } } - public static void validateParsedAuthority(URI uri) { + /** + * Verifies that the authority part of a uProtocol URI complies with the uProtocol specification. + * + * @param uri The URI to validate. + * @throws NullPointerException if uri is {@code null}. + * @throws IllegalArgumentException if the authority part of the URI does not comply with the + * uProtocol specification. + * @return The validated authority part of the URI. + */ + // [impl->dsn~uri-authority-name-length~1] + // [impl->dsn~uri-host-only~2] + public static String validateAuthority(URI uri) { Objects.requireNonNull(uri, "URI must not be null"); - if (uri.getPort() != -1) { - throw new IllegalArgumentException("uProtocol URI must not contain port"); + String authority; + try { + // this should work if authority is server-based, i.e. contains a host, literal IP or IPv4 address + var uriWithServerAuthority = uri.parseServerAuthority(); + if (uriWithServerAuthority.getPort() != -1) { + throw new IllegalArgumentException("uProtocol URI must not contain port"); + } + if (uriWithServerAuthority.getUserInfo() != null) { + throw new IllegalArgumentException("uProtocol URI must not contain user info"); + } + authority = uriWithServerAuthority.getHost(); + if (authority != null && authority.startsWith("[") && authority.endsWith("]")) { + // this is an IPv6 literal address + return authority; + } + } catch (URISyntaxException e) { + // the authority is not server-based but might still be valid according to the UUri spec, + authority = uri.getAuthority(); } - if (uri.getUserInfo() != null) { - throw new IllegalArgumentException("uProtocol URI must not contain user info"); + // make sure that authority name either is the wildcard authority or contains allowed characters only + if (authority != null && !"*".equals(authority) && !AUTHORITY_PATTERN.matcher(authority).matches()) { + throw new IllegalArgumentException("uProtocol URI authority contains invalid characters"); } - Optional.ofNullable(uri.getAuthority()).ifPresent(authority -> { - if (authority.length() > 128) { - throw new IllegalArgumentException("Authority name exceeds maximum length of 128 characters"); - } - }); - // TODO: make sure that authority name only consists of allowed characters + // and does not exceed maximum length + if (authority != null && authority.length() > AUTHORITY_NAME_MAX_LENGTH) { + throw new IllegalArgumentException("uProtocol URI authority must not exceed %d characters" + .formatted(AUTHORITY_NAME_MAX_LENGTH)); + } + + return authority; } /** @@ -283,9 +320,21 @@ public static boolean hasWildcard(UUri uri) { * * @param sourceFilter The source filter URI to verify. * @param sinkFilter The optional sink filter URI to verify. + * @throws NullPointerException if any of the arguments are {@code null}. * @throws UStatusException if the given URIs cannot be used as filter criteria. */ public static void verifyFilterCriteria(UUri sourceFilter, Optional sinkFilter) { + Objects.requireNonNull(sourceFilter); + Objects.requireNonNull(sinkFilter); + try { + validate(sourceFilter); + sinkFilter.ifPresent(UriValidator::validate); + } catch (IllegalArgumentException e) { + throw new UStatusException( + UCode.INVALID_ARGUMENT, + "source and sink filters must be valid uProtocol URIs", + e); + } sinkFilter.ifPresentOrElse( filter -> { if (isNotificationDestination(filter) && isNotificationDestination(sourceFilter)) { diff --git a/src/test/java/org/eclipse/uprotocol/uri/serializer/UriSerializerTest.java b/src/test/java/org/eclipse/uprotocol/uri/serializer/UriSerializerTest.java index 2173b2b..4edde79 100644 --- a/src/test/java/org/eclipse/uprotocol/uri/serializer/UriSerializerTest.java +++ b/src/test/java/org/eclipse/uprotocol/uri/serializer/UriSerializerTest.java @@ -12,6 +12,7 @@ */ package org.eclipse.uprotocol.uri.serializer; +import org.eclipse.uprotocol.uri.validator.UriValidator; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -22,8 +23,6 @@ class UriSerializerTest { - private static final int AUTHORITY_NAME_MAX_LENGTH = 128; - @Test @DisplayName("Test serializing a null UUri fails") void testSerializingANullUuri() { @@ -41,11 +40,11 @@ void testDeserializingANullUuriFails() { @DisplayName("Test deserializing a UUri with authority name exceeding max length fails") // [utest->dsn~uri-authority-name-length~1] void testDeserializeRejectsAuthorityNameExceedingMaxLength() { - String authority = "a".repeat(AUTHORITY_NAME_MAX_LENGTH); + String authority = "a".repeat(UriValidator.AUTHORITY_NAME_MAX_LENGTH); String validUri = "up://%s/ABCD/1/1001".formatted(authority); assertDoesNotThrow(() -> UriSerializer.deserialize(validUri)); - authority = "a".repeat(AUTHORITY_NAME_MAX_LENGTH + 1); + authority = "a".repeat(UriValidator.AUTHORITY_NAME_MAX_LENGTH + 1); var invalidUri = "up://%s/ABCD/1/1001".formatted(authority); assertThrows(IllegalArgumentException.class, () -> UriSerializer.deserialize(invalidUri)); } diff --git a/src/test/java/org/eclipse/uprotocol/uri/validator/UriValidatorTest.java b/src/test/java/org/eclipse/uprotocol/uri/validator/UriValidatorTest.java index 6735f24..1f67a1f 100644 --- a/src/test/java/org/eclipse/uprotocol/uri/validator/UriValidatorTest.java +++ b/src/test/java/org/eclipse/uprotocol/uri/validator/UriValidatorTest.java @@ -18,7 +18,9 @@ import org.eclipse.uprotocol.v1.UCode; import org.eclipse.uprotocol.v1.UUri; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.MethodSource; import static org.junit.Assert.assertEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -26,8 +28,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.util.Arrays; import java.util.Optional; +import java.util.stream.Stream; class UriValidatorTest { @@ -36,9 +38,12 @@ class UriValidatorTest { authorityName, ueId, version, resourceId, should succeed *, -1, 0xFF, 0xFFFF, true myhost, 0x0000_0A1B, 0x01, 0x2341, true + 192.168.1.1, 0x0000_0A1B, 0x01, 0x2341, true + [2001::7], 0x0000_0A1B, 0x01, 0x2341, true invalid<<[], 0x0000_0A1B, 0x01, 0x2341, false myhost:5555, 0x0000_0A1B, 0x01, 0x2341, false user:passwd@myhost, 0x0000_0A1B, 0x01, 0x2341, false + MYHOST, 0x0000_0A1B, 0x01, 0x2341, false myhost, 0x0000_0A1B, -1, 0x2341, false myhost, 0x0000_0A1B, 0x100, 0x2341, false myhost, 0x0000_0A1B, 0x01, -1, false @@ -73,10 +78,9 @@ void testValidate( 129, false """) void testValidateFailsForAuthorityExceedingMaxLength(int authorityNameLength, boolean shouldSucceed) { - var authorityName = new char[authorityNameLength]; - Arrays.fill(authorityName, 'A'); + var authorityName = "a".repeat(authorityNameLength); var uri = UUri.newBuilder() - .setAuthorityName(new String(authorityName)) + .setAuthorityName(authorityName) .setUeId(0x1234) .setUeVersionMajor(0x01) .setResourceId(0x0001) @@ -170,33 +174,79 @@ void testHasWildcard(String uri, boolean shouldSucceed) { } } + static Stream verifyFilterCriteriaProvider() { + var templateUriA = UUri.newBuilder() + .setAuthorityName("vehicle1") + .setUeId(0xaa) + .setUeVersionMajor(0x01) + .build(); + var templateUriB = UUri.newBuilder() + .setAuthorityName("vehicle2") + .setUeId(0xbb) + .setUeVersionMajor(0x01) + .build(); + return Stream.of( + // source has authority name with upper-case letters + Arguments.of( + UUri.newBuilder(templateUriA).setAuthorityName("VEHICLE1").setResourceId(0x9000).build(), + Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x0000).build()), + false), + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0xFFFF).build(), + Optional.of(UUri.newBuilder(templateUriB).setResourceId(0xFFFF).build()), + true), + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0x9000).build(), + Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x0000).build()), + true), + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0x0000).build(), + Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x0001).build()), + true), + // source and sink both have resource ID 0 + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0x0000).build(), + Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x0000).build()), + false), + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0xFFFF).build(), + Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x001a).build()), + true), + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0x0000).build(), + Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x001a).build()), + true), + // sink is RPC but source has invalid resource ID + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0x00cc).build(), + Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x001a).build()), + false), + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0x9000).build(), + Optional.empty(), + true), + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0xFFFF).build(), + Optional.empty(), + true), + // sink is empty but source has non-topic resource ID + Arguments.of( + UUri.newBuilder(templateUriA).setResourceId(0x00cc).build(), + Optional.empty(), + false) + ); + } + @ParameterizedTest(name = "Test verifyFilterCriteria: {index} {arguments}") - @CsvSource(useHeadersInDisplayName = true, textBlock = """ - source, sink, should fail - //vehicle1/AA/1/FFFF, //vehicle2/BB/1/FFFF, false - //vehicle1/AA/1/9000, //vehicle2/BB/1/0, false - //vehicle1/AA/1/0, //vehicle2/BB/1/1, false - # source and sink both have resource ID 0 - //vehicle1/AA/1/0, //vehicle2/BB/1/0, true - //vehicle1/AA/1/FFFF, //vehicle2/BB/1/1A, false - //vehicle1/AA/1/0, //vehicle2/BB/1/1A, false - # sink is RPC but source has invalid resource ID - //vehicle1/AA/1/CC, //vehicle2/BB/1/1A, true - //vehicle1/AA/1/9000, , false - //vehicle1/AA/1/FFFF, , false - # sink is empty but source has non-topic resource ID - //vehicle1/AA/1/CC, , true - """) - void testVerifyFilterCriteriaFails(String source, String sink, boolean shouldFail) { - var sourceFilter = UriSerializer.deserialize(source); - Optional sinkFilter = sink != null ? Optional.of(UriSerializer.deserialize(sink)) : Optional.empty(); - if (shouldFail) { + @MethodSource("verifyFilterCriteriaProvider") + void testVerifyFilterCriteriaFails(UUri sourceFilter, Optional sinkFilter, boolean shouldSucceed) { + if (shouldSucceed) { + assertDoesNotThrow(() -> UriValidator.verifyFilterCriteria(sourceFilter, sinkFilter)); + } else { UStatusException exception = assertThrows( UStatusException.class, () -> UriValidator.verifyFilterCriteria(sourceFilter, sinkFilter)); assertEquals(UCode.INVALID_ARGUMENT, exception.getCode()); - } else { - assertDoesNotThrow(() -> UriValidator.verifyFilterCriteria(sourceFilter, sinkFilter)); } } } diff --git a/src/test/resources/features/uuri_uri_serialization.feature b/src/test/resources/features/uuri_uri_serialization.feature index 0c8a330..b007428 100644 --- a/src/test/resources/features/uuri_uri_serialization.feature +++ b/src/test/resources/features/uuri_uri_serialization.feature @@ -78,6 +78,7 @@ Feature: String representation of endpoint identfiers (UUri) | "xy://vcu.my_vin/101/1/A" | unsupported schema | | "//vcu.my_vin/101/1/A?foo=bar" | URI with query | | "//vcu.my_vin/101/1/A#foo" | URI with fragment | + | "//VCU.my-vin/101/1/A" | server-based authority with upper-case letters | | "//vcu.my-vin:1516/101/1/A" | server-based authority with port | | "//user:pwd@vcu.my-vin/101/1/A" | server-based authority with user info | | "//[2001:db87aa::8]/101/1/A" | invalid IP literal authority |