Skip to content

Commit 43fcbc7

Browse files
sophokles73sophokles73
committed
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.
1 parent 6489c89 commit 43fcbc7

File tree

5 files changed

+144
-71
lines changed

5 files changed

+144
-71
lines changed

‎src/main/java/org/eclipse/uprotocol/uri/serializer/UriSerializer.java‎

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,8 @@
1313
package org.eclipse.uprotocol.uri.serializer;
1414

1515
import java.net.URI;
16-
import java.net.URISyntaxException;
1716
import java.util.Objects;
1817
import java.util.Optional;
19-
import java.util.regex.Pattern;
2018

2119
import org.eclipse.uprotocol.uri.validator.UriValidator;
2220
import org.eclipse.uprotocol.v1.UUri;
@@ -35,8 +33,6 @@ public final class UriSerializer {
3533
*/
3634
public static final String SCHEME_UP = "up";
3735

38-
private static final Pattern AUTHORITY_PATTERN = Pattern.compile("^[a-z0-9-._~]{0,128}$");
39-
4036
private UriSerializer() {
4137
// prevent instantiation
4238
}
@@ -105,12 +101,10 @@ public static String serialize(UUri uuri, boolean includeScheme) {
105101
* @throws NullPointerException if the URI is null.
106102
* @throws IllegalArgumentException if the URI is invalid.
107103
*/
108-
// [impl->dsn~uri-authority-name-length~1]
109104
// [impl->dsn~uri-scheme~1]
110-
// [impl->dsn~uri-host-only~2]
111-
// [impl->dsn~uri-authority-mapping~1]
112105
// [impl->dsn~uri-path-mapping~1]
113106
// [impl->req~uri-serialization~1]
107+
// [impl->dsn~uri-authority-mapping~1]
114108
public static UUri deserialize(String uProtocolUri) {
115109
Objects.requireNonNull(uProtocolUri);
116110
final var parsedUri = URI.create(uProtocolUri);
@@ -125,12 +119,10 @@ public static UUri deserialize(String uProtocolUri) {
125119
* @throws NullPointerException if the URI is null.
126120
* @throws IllegalArgumentException if the URI is invalid.
127121
*/
128-
// [impl->dsn~uri-authority-name-length~1]
129122
// [impl->dsn~uri-scheme~1]
130-
// [impl->dsn~uri-host-only~2]
131-
// [impl->dsn~uri-authority-mapping~1]
132123
// [impl->dsn~uri-path-mapping~1]
133124
// [impl->req~uri-serialization~1]
125+
// [impl->dsn~uri-authority-mapping~1]
134126
public static UUri deserialize(URI uProtocolUri) {
135127
Objects.requireNonNull(uProtocolUri);
136128

@@ -143,25 +135,7 @@ public static UUri deserialize(URI uProtocolUri) {
143135
if (uProtocolUri.getFragment() != null) {
144136
throw new IllegalArgumentException("uProtocol URI must not contain fragment");
145137
}
146-
147-
String authority;
148-
try {
149-
// this should work if authority is server-based (i.e. contains a host)
150-
var uriWithServerAuthority = uProtocolUri.parseServerAuthority();
151-
// we can then verify that the authority does neither contain user info nor port
152-
UriValidator.validateParsedAuthority(uriWithServerAuthority);
153-
authority = uriWithServerAuthority.getHost();
154-
} catch (URISyntaxException e) {
155-
// the authority is not server-based but might still be valid according to the UUri spec,
156-
// we just need to make sure that it either is the wildcard authority or contains allowed
157-
// characters only
158-
authority = uProtocolUri.getAuthority();
159-
if (authority != null && !"*".equals(authority) && !AUTHORITY_PATTERN.matcher(authority).matches()) {
160-
throw new IllegalArgumentException(
161-
"uProtocol URI authority contains invalid characters",
162-
e);
163-
}
164-
}
138+
String authority = UriValidator.validateAuthority(uProtocolUri);
165139

166140
final var pathSegments = uProtocolUri.getPath().split("/");
167141
if (pathSegments.length != 4) {

‎src/main/java/org/eclipse/uprotocol/uri/validator/UriValidator.java‎

Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.net.URISyntaxException;
1717
import java.util.Objects;
1818
import java.util.Optional;
19+
import java.util.regex.Pattern;
1920

2021
import org.eclipse.uprotocol.communication.UStatusException;
2122
import org.eclipse.uprotocol.transport.UTransport;
@@ -28,6 +29,13 @@
2829
*/
2930
public final class UriValidator {
3031

32+
/**
33+
* The maximum length of an authority name.
34+
*/
35+
public static final int AUTHORITY_NAME_MAX_LENGTH = 128;
36+
37+
private static final Pattern AUTHORITY_PATTERN = Pattern.compile("^[a-z0-9-._~]*$");
38+
3139
private UriValidator() {
3240
// prevent instantiation
3341
}
@@ -57,7 +65,7 @@ public static void validate(UUri uuri) {
5765
.ifPresent(name -> {
5866
try {
5967
var uri = new URI(null, name, null, null, null);
60-
validateParsedAuthority(uri);
68+
validateAuthority(uri);
6169
} catch (URISyntaxException e) {
6270
throw new IllegalArgumentException("Invalid authority name", e);
6371
}
@@ -84,21 +92,50 @@ public static void validateResourceId(int resourceId) {
8492
}
8593
}
8694

87-
public static void validateParsedAuthority(URI uri) {
95+
/**
96+
* Verifies that the authority part of a uProtocol URI complies with the uProtocol specification.
97+
*
98+
* @param uri The URI to validate.
99+
* @throws NullPointerException if uri is {@code null}.
100+
* @throws IllegalArgumentException if the authority part of the URI does not comply with the
101+
* uProtocol specification.
102+
* @return The validated authority part of the URI.
103+
*/
104+
// [impl->dsn~uri-authority-name-length~1]
105+
// [impl->dsn~uri-host-only~2]
106+
public static String validateAuthority(URI uri) {
88107
Objects.requireNonNull(uri, "URI must not be null");
89108

90-
if (uri.getPort() != -1) {
91-
throw new IllegalArgumentException("uProtocol URI must not contain port");
109+
String authority;
110+
try {
111+
// this should work if authority is server-based, i.e. contains a host, literal IP or IPv4 address
112+
var uriWithServerAuthority = uri.parseServerAuthority();
113+
if (uriWithServerAuthority.getPort() != -1) {
114+
throw new IllegalArgumentException("uProtocol URI must not contain port");
115+
}
116+
if (uriWithServerAuthority.getUserInfo() != null) {
117+
throw new IllegalArgumentException("uProtocol URI must not contain user info");
118+
}
119+
authority = uriWithServerAuthority.getHost();
120+
if (authority != null && authority.startsWith("[") && authority.endsWith("]")) {
121+
// this is an IPv6 literal address
122+
return authority;
123+
}
124+
} catch (URISyntaxException e) {
125+
// the authority is not server-based but might still be valid according to the UUri spec,
126+
authority = uri.getAuthority();
92127
}
93-
if (uri.getUserInfo() != null) {
94-
throw new IllegalArgumentException("uProtocol URI must not contain user info");
128+
// make sure that authority name either is the wildcard authority or contains allowed characters only
129+
if (authority != null && !"*".equals(authority) && !AUTHORITY_PATTERN.matcher(authority).matches()) {
130+
throw new IllegalArgumentException("uProtocol URI authority contains invalid characters");
95131
}
96-
Optional.ofNullable(uri.getAuthority()).ifPresent(authority -> {
97-
if (authority.length() > 128) {
98-
throw new IllegalArgumentException("Authority name exceeds maximum length of 128 characters");
99-
}
100-
});
101-
// TODO: make sure that authority name only consists of allowed characters
132+
// and does not exceed maximum length
133+
if (authority != null && authority.length() > AUTHORITY_NAME_MAX_LENGTH) {
134+
throw new IllegalArgumentException("uProtocol URI authority must not exceed %d characters"
135+
.formatted(AUTHORITY_NAME_MAX_LENGTH));
136+
}
137+
138+
return authority;
102139
}
103140

104141
/**
@@ -283,9 +320,21 @@ public static boolean hasWildcard(UUri uri) {
283320
*
284321
* @param sourceFilter The source filter URI to verify.
285322
* @param sinkFilter The optional sink filter URI to verify.
323+
* @throws NullPointerException if any of the arguments are {@code null}.
286324
* @throws UStatusException if the given URIs cannot be used as filter criteria.
287325
*/
288326
public static void verifyFilterCriteria(UUri sourceFilter, Optional<UUri> sinkFilter) {
327+
Objects.requireNonNull(sourceFilter);
328+
Objects.requireNonNull(sinkFilter);
329+
try {
330+
validate(sourceFilter);
331+
sinkFilter.ifPresent(UriValidator::validate);
332+
} catch (IllegalArgumentException e) {
333+
throw new UStatusException(
334+
UCode.INVALID_ARGUMENT,
335+
"source and sink filters must be valid uProtocol URIs",
336+
e);
337+
}
289338
sinkFilter.ifPresentOrElse(
290339
filter -> {
291340
if (isNotificationDestination(filter) && isNotificationDestination(sourceFilter)) {

‎src/test/java/org/eclipse/uprotocol/uri/serializer/UriSerializerTest.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
package org.eclipse.uprotocol.uri.serializer;
1414

15+
import org.eclipse.uprotocol.uri.validator.UriValidator;
1516
import org.junit.jupiter.api.DisplayName;
1617
import org.junit.jupiter.api.Test;
1718

@@ -22,8 +23,6 @@
2223

2324
class UriSerializerTest {
2425

25-
private static final int AUTHORITY_NAME_MAX_LENGTH = 128;
26-
2726
@Test
2827
@DisplayName("Test serializing a null UUri fails")
2928
void testSerializingANullUuri() {
@@ -41,11 +40,11 @@ void testDeserializingANullUuriFails() {
4140
@DisplayName("Test deserializing a UUri with authority name exceeding max length fails")
4241
// [utest->dsn~uri-authority-name-length~1]
4342
void testDeserializeRejectsAuthorityNameExceedingMaxLength() {
44-
String authority = "a".repeat(AUTHORITY_NAME_MAX_LENGTH);
43+
String authority = "a".repeat(UriValidator.AUTHORITY_NAME_MAX_LENGTH);
4544
String validUri = "up://%s/ABCD/1/1001".formatted(authority);
4645
assertDoesNotThrow(() -> UriSerializer.deserialize(validUri));
4746

48-
authority = "a".repeat(AUTHORITY_NAME_MAX_LENGTH + 1);
47+
authority = "a".repeat(UriValidator.AUTHORITY_NAME_MAX_LENGTH + 1);
4948
var invalidUri = "up://%s/ABCD/1/1001".formatted(authority);
5049
assertThrows(IllegalArgumentException.class, () -> UriSerializer.deserialize(invalidUri));
5150
}

‎src/test/java/org/eclipse/uprotocol/uri/validator/UriValidatorTest.java‎

Lines changed: 76 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,18 @@
1818
import org.eclipse.uprotocol.v1.UCode;
1919
import org.eclipse.uprotocol.v1.UUri;
2020
import org.junit.jupiter.params.ParameterizedTest;
21+
import org.junit.jupiter.params.provider.Arguments;
2122
import org.junit.jupiter.params.provider.CsvSource;
23+
import org.junit.jupiter.params.provider.MethodSource;
2224

2325
import static org.junit.Assert.assertEquals;
2426
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
2527
import static org.junit.jupiter.api.Assertions.assertFalse;
2628
import static org.junit.jupiter.api.Assertions.assertThrows;
2729
import static org.junit.jupiter.api.Assertions.assertTrue;
2830

29-
import java.util.Arrays;
3031
import java.util.Optional;
32+
import java.util.stream.Stream;
3133

3234
class UriValidatorTest {
3335

@@ -36,9 +38,12 @@ class UriValidatorTest {
3638
authorityName, ueId, version, resourceId, should succeed
3739
*, -1, 0xFF, 0xFFFF, true
3840
myhost, 0x0000_0A1B, 0x01, 0x2341, true
41+
192.168.1.1, 0x0000_0A1B, 0x01, 0x2341, true
42+
[2001::7], 0x0000_0A1B, 0x01, 0x2341, true
3943
invalid<<[], 0x0000_0A1B, 0x01, 0x2341, false
4044
myhost:5555, 0x0000_0A1B, 0x01, 0x2341, false
4145
user:passwd@myhost, 0x0000_0A1B, 0x01, 0x2341, false
46+
MYHOST, 0x0000_0A1B, 0x01, 0x2341, false
4247
myhost, 0x0000_0A1B, -1, 0x2341, false
4348
myhost, 0x0000_0A1B, 0x100, 0x2341, false
4449
myhost, 0x0000_0A1B, 0x01, -1, false
@@ -73,10 +78,9 @@ void testValidate(
7378
129, false
7479
""")
7580
void testValidateFailsForAuthorityExceedingMaxLength(int authorityNameLength, boolean shouldSucceed) {
76-
var authorityName = new char[authorityNameLength];
77-
Arrays.fill(authorityName, 'A');
81+
var authorityName = "a".repeat(authorityNameLength);
7882
var uri = UUri.newBuilder()
79-
.setAuthorityName(new String(authorityName))
83+
.setAuthorityName(authorityName)
8084
.setUeId(0x1234)
8185
.setUeVersionMajor(0x01)
8286
.setResourceId(0x0001)
@@ -170,33 +174,79 @@ void testHasWildcard(String uri, boolean shouldSucceed) {
170174
}
171175
}
172176

177+
static Stream<Arguments> verifyFilterCriteriaProvider() {
178+
var templateUriA = UUri.newBuilder()
179+
.setAuthorityName("vehicle1")
180+
.setUeId(0xaa)
181+
.setUeVersionMajor(0x01)
182+
.build();
183+
var templateUriB = UUri.newBuilder()
184+
.setAuthorityName("vehicle2")
185+
.setUeId(0xbb)
186+
.setUeVersionMajor(0x01)
187+
.build();
188+
return Stream.of(
189+
// source has authority name with upper-case letters
190+
Arguments.of(
191+
UUri.newBuilder(templateUriA).setAuthorityName("VEHICLE1").setResourceId(0x9000).build(),
192+
Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x0000).build()),
193+
false),
194+
Arguments.of(
195+
UUri.newBuilder(templateUriA).setResourceId(0xFFFF).build(),
196+
Optional.of(UUri.newBuilder(templateUriB).setResourceId(0xFFFF).build()),
197+
true),
198+
Arguments.of(
199+
UUri.newBuilder(templateUriA).setResourceId(0x9000).build(),
200+
Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x0000).build()),
201+
true),
202+
Arguments.of(
203+
UUri.newBuilder(templateUriA).setResourceId(0x0000).build(),
204+
Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x0001).build()),
205+
true),
206+
// source and sink both have resource ID 0
207+
Arguments.of(
208+
UUri.newBuilder(templateUriA).setResourceId(0x0000).build(),
209+
Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x0000).build()),
210+
false),
211+
Arguments.of(
212+
UUri.newBuilder(templateUriA).setResourceId(0xFFFF).build(),
213+
Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x001a).build()),
214+
true),
215+
Arguments.of(
216+
UUri.newBuilder(templateUriA).setResourceId(0x0000).build(),
217+
Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x001a).build()),
218+
true),
219+
// sink is RPC but source has invalid resource ID
220+
Arguments.of(
221+
UUri.newBuilder(templateUriA).setResourceId(0x00cc).build(),
222+
Optional.of(UUri.newBuilder(templateUriB).setResourceId(0x001a).build()),
223+
false),
224+
Arguments.of(
225+
UUri.newBuilder(templateUriA).setResourceId(0x9000).build(),
226+
Optional.empty(),
227+
true),
228+
Arguments.of(
229+
UUri.newBuilder(templateUriA).setResourceId(0xFFFF).build(),
230+
Optional.empty(),
231+
true),
232+
// sink is empty but source has non-topic resource ID
233+
Arguments.of(
234+
UUri.newBuilder(templateUriA).setResourceId(0x00cc).build(),
235+
Optional.empty(),
236+
false)
237+
);
238+
}
239+
173240
@ParameterizedTest(name = "Test verifyFilterCriteria: {index} {arguments}")
174-
@CsvSource(useHeadersInDisplayName = true, textBlock = """
175-
source, sink, should fail
176-
//vehicle1/AA/1/FFFF, //vehicle2/BB/1/FFFF, false
177-
//vehicle1/AA/1/9000, //vehicle2/BB/1/0, false
178-
//vehicle1/AA/1/0, //vehicle2/BB/1/1, false
179-
# source and sink both have resource ID 0
180-
//vehicle1/AA/1/0, //vehicle2/BB/1/0, true
181-
//vehicle1/AA/1/FFFF, //vehicle2/BB/1/1A, false
182-
//vehicle1/AA/1/0, //vehicle2/BB/1/1A, false
183-
# sink is RPC but source has invalid resource ID
184-
//vehicle1/AA/1/CC, //vehicle2/BB/1/1A, true
185-
//vehicle1/AA/1/9000, , false
186-
//vehicle1/AA/1/FFFF, , false
187-
# sink is empty but source has non-topic resource ID
188-
//vehicle1/AA/1/CC, , true
189-
""")
190-
void testVerifyFilterCriteriaFails(String source, String sink, boolean shouldFail) {
191-
var sourceFilter = UriSerializer.deserialize(source);
192-
Optional<UUri> sinkFilter = sink != null ? Optional.of(UriSerializer.deserialize(sink)) : Optional.empty();
193-
if (shouldFail) {
241+
@MethodSource("verifyFilterCriteriaProvider")
242+
void testVerifyFilterCriteriaFails(UUri sourceFilter, Optional<UUri> sinkFilter, boolean shouldSucceed) {
243+
if (shouldSucceed) {
244+
assertDoesNotThrow(() -> UriValidator.verifyFilterCriteria(sourceFilter, sinkFilter));
245+
} else {
194246
UStatusException exception = assertThrows(
195247
UStatusException.class,
196248
() -> UriValidator.verifyFilterCriteria(sourceFilter, sinkFilter));
197249
assertEquals(UCode.INVALID_ARGUMENT, exception.getCode());
198-
} else {
199-
assertDoesNotThrow(() -> UriValidator.verifyFilterCriteria(sourceFilter, sinkFilter));
200250
}
201251
}
202252
}

‎src/test/resources/features/uuri_uri_serialization.feature‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ Feature: String representation of endpoint identfiers (UUri)
7878
| "xy://vcu.my_vin/101/1/A" | unsupported schema |
7979
| "//vcu.my_vin/101/1/A?foo=bar" | URI with query |
8080
| "//vcu.my_vin/101/1/A#foo" | URI with fragment |
81+
| "//VCU.my-vin/101/1/A" | server-based authority with upper-case letters |
8182
| "//vcu.my-vin:1516/101/1/A" | server-based authority with port |
8283
| "//user:pwd@vcu.my-vin/101/1/A" | server-based authority with user info |
8384
| "//[2001:db87aa::8]/101/1/A" | invalid IP literal authority |

0 commit comments

Comments
 (0)