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 |