Skip to content

Commit

Permalink
Throw IllegalStateException for invalid port in HierarchicalUriCompon…
Browse files Browse the repository at this point in the history
…ents

Prior to this commit, getPort() in HierarchicalUriComponents threw a
NumberFormatException for an invalid port supplied as a String, which
was inconsistent with exception handling elsewhere in the class as well
as within the same method.

This commit introduces a try-catch block in getPort() to consistently
throw IllegalStateExceptions for ports that cannot be parsed.

Closes gh-28521
  • Loading branch information
sbrannen committed May 25, 2022
1 parent aa06a09 commit a221835
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 8 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -46,6 +46,7 @@
* @author Juergen Hoeller
* @author Rossen Stoyanchev
* @author Phillip Webb
* @author Sam Brannen
* @since 3.1.3
* @see <a href="https://tools.ietf.org/html/rfc3986#section-1.2.3">Hierarchical URIs</a>
*/
Expand Down Expand Up @@ -192,7 +193,12 @@ else if (this.port.contains("{")) {
throw new IllegalStateException(
"The port contains a URI variable but has not been expanded yet: " + this.port);
}
return Integer.parseInt(this.port);
try {
return Integer.parseInt(this.port);
}
catch (NumberFormatException ex) {
throw new IllegalStateException("The port must be an integer: " + this.port);
}
}

@Override
Expand Down
Expand Up @@ -40,7 +40,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;

/**
* Unit tests for {@link UriComponentsBuilder}.
Expand Down Expand Up @@ -1314,11 +1314,13 @@ void validPort() {

@Test
void verifyInvalidPort() {
String url = "http://localhost:port/path";
assertThatThrownBy(() -> UriComponentsBuilder.fromUriString(url).build().toUri())
.isInstanceOf(NumberFormatException.class);
assertThatThrownBy(() -> UriComponentsBuilder.fromHttpUrl(url).build().toUri())
.isInstanceOf(NumberFormatException.class);
String url = "http://localhost:XXX/path";
assertThatIllegalStateException()
.isThrownBy(() -> UriComponentsBuilder.fromUriString(url).build().toUri())
.withMessage("The port must be an integer: XXX");
assertThatIllegalStateException()
.isThrownBy(() -> UriComponentsBuilder.fromHttpUrl(url).build().toUri())
.withMessage("The port must be an integer: XXX");
}

@Test // gh-27039
Expand Down
Expand Up @@ -30,6 +30,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.springframework.web.util.UriComponentsBuilder.fromHttpUrl;
import static org.springframework.web.util.UriComponentsBuilder.fromUriString;

/**
Expand Down Expand Up @@ -151,6 +152,29 @@ void port() {
assertThat(uri4.toUriString()).isEqualTo("https://example.com:8080/bar");
}

@Test // gh-28521
void invalidPort() {
assertExceptionsForInvalidPort(fromUriString("https://example.com:XXX/bar").build());
assertExceptionsForInvalidPort(fromUriString("https://example.com/bar").port("XXX").build());
assertExceptionsForInvalidPort(fromHttpUrl("https://example.com:XXX/bar").build());
assertExceptionsForInvalidPort(fromHttpUrl("https://example.com/bar").port("XXX").build());
}

private void assertExceptionsForInvalidPort(UriComponents uriComponents) {
assertThatIllegalStateException()
.isThrownBy(uriComponents::getPort)
.withMessage("The port must be an integer: XXX");
assertThatIllegalStateException()
.isThrownBy(uriComponents::toUri)
.withMessage("The port must be an integer: XXX");
assertThatIllegalStateException()
.isThrownBy(uriComponents::toUriString)
.withMessage("The port must be an integer: XXX");
assertThatIllegalStateException()
.isThrownBy(uriComponents::toString)
.withMessage("The port must be an integer: XXX");
}

@Test
void expandEncoded() {
assertThatIllegalStateException().isThrownBy(() ->
Expand Down

0 comments on commit a221835

Please sign in to comment.