Skip to content

Commit

Permalink
HTTPCORE-756 - Enforced the requirement that "https" URIs must not h…
Browse files Browse the repository at this point in the history
…ave an empty host identifier.

- Adjusted URIBuilder's toString() method to return "invalid://" for URIs that violate this rule, ensuring compliance with the RFC's directive to reject such URIs as invalid.
  • Loading branch information
arturobernalg committed Aug 26, 2023
1 parent 4d6d9f5 commit 9ca51ab
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 24 deletions.
14 changes: 11 additions & 3 deletions httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.NameValuePair;
import org.apache.hc.core5.http.URIScheme;
import org.apache.hc.core5.http.message.BasicNameValuePair;
import org.apache.hc.core5.http.message.ParserCursor;
import org.apache.hc.core5.util.Args;
Expand Down Expand Up @@ -313,11 +314,14 @@ public URI build() throws URISyntaxException {
return new URI(buildString());
}

private String buildString() {
private String buildString() throws URISyntaxException {
final StringBuilder sb = new StringBuilder();
if (this.scheme != null) {
sb.append(this.scheme).append(':');
}
if (URIScheme.HTTPS.same(scheme) && (host == null || host.isEmpty())) {
throw new URISyntaxException(sb.toString(), "https URI cannot have an empty host identifier");
}
if (this.encodedSchemeSpecificPart != null) {
sb.append(this.encodedSchemeSpecificPart);
} else {
Expand Down Expand Up @@ -1075,11 +1079,15 @@ public URIBuilder optimize() {
/**
* Converts this instance to a URI string.
*
* @return this instance to a URI string.
* @return this instance to a URI string, or {@code invalid://} {@link String} if building the string fails due to an invalid {@link URI}.
*/
@Override
public String toString() {
return buildString();
try {
return buildString();
} catch (final URISyntaxException e) {
return "invalid://";
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import java.time.Duration;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -63,7 +65,7 @@ public void testCompleted() throws Exception {
Mockito.verify(callback, Mockito.never()).cancelled();

Assertions.assertSame(result, future.get());
Assertions.assertTrue(future.isDone());
assertTrue(future.isDone());
Assertions.assertFalse(future.isCancelled());

}
Expand All @@ -84,7 +86,7 @@ public void testCompletedWithTimeout() throws Exception {
Mockito.verify(callback, Mockito.never()).cancelled();

Assertions.assertSame(result, future.get(1, TimeUnit.MILLISECONDS));
Assertions.assertTrue(future.isDone());
assertTrue(future.isDone());
Assertions.assertFalse(future.isCancelled());
}

Expand All @@ -105,7 +107,7 @@ public void testFailed() throws Exception {
} catch (final ExecutionException ex) {
Assertions.assertSame(boom, ex.getCause());
}
Assertions.assertTrue(future.isDone());
assertTrue(future.isDone());
Assertions.assertFalse(future.isCancelled());
}

Expand All @@ -123,8 +125,8 @@ public void testCancelled() throws Exception {
Mockito.verify(callback).cancelled();

assertThrows(CancellationException.class, future::get);
Assertions.assertTrue(future.isDone());
Assertions.assertTrue(future.isCancelled());
assertTrue(future.isDone());
assertTrue(future.isCancelled());
}

@Test
Expand All @@ -142,7 +144,7 @@ public void testAsyncCompleted() throws Exception {
t.setDaemon(true);
t.start();
Assertions.assertSame(result, future.get(60, TimeUnit.SECONDS));
Assertions.assertTrue(future.isDone());
assertTrue(future.isDone());
Assertions.assertFalse(future.isCancelled());
}

Expand All @@ -165,7 +167,7 @@ public void testAsyncFailed() throws Exception {
} catch (final ExecutionException ex) {
Assertions.assertSame(boom, ex.getCause());
}
Assertions.assertTrue(future.isDone());
assertTrue(future.isDone());
Assertions.assertFalse(future.isCancelled());
}

Expand Down Expand Up @@ -252,7 +254,7 @@ public void cancelled() {
}

@Test
void testGetWithTimeout() {
void testGetWithTimeout() throws InterruptedException, ExecutionException {
final FutureCallback<String> callback = new FutureCallback<String>() {
@Override
public void completed(final String result) {
Expand All @@ -271,23 +273,19 @@ public void cancelled() {
};

final BasicFuture<String> future = new BasicFuture<>(callback);
final CountDownLatch latch = new CountDownLatch(1);

new Thread(() -> {
try {
Thread.sleep(100); // This simulates the delay in completing the future.
future.completed("test");
} catch (final InterruptedException e) {
future.failed(e);
}
future.completed("test");
latch.countDown(); // Signal that the future is completed.
}).start();

assertTimeoutPreemptively(Duration.ofMillis(200), () -> {
try {
assertEquals("test", future.get(1, TimeUnit.SECONDS));
} catch (final ExecutionException | TimeoutException e) {
fail("Test failed due to exception: " + e.getMessage());
}
});
// Await the completion of the future, but no more than 500ms.
boolean completedInTime = latch.await(500, TimeUnit.MILLISECONDS);
assertTrue(completedInTime, "Future did not complete in the expected time.");

assertEquals("test", future.get());
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -952,4 +952,18 @@ public void testBuilderWithUnbracketedIpv6Host() throws Exception {
Assertions.assertEquals("/path", builder.getPath());
Assertions.assertEquals("/path", uri.getPath());
}

@Test
public void testHttpsUriWithEmptyHost() {
final URIBuilder uribuilder = new URIBuilder()
.setScheme("https")
.setUserInfo("stuff")
.setHost("")
.setPort(80)
.setPath("/some stuff")
.setParameter("param", "stuff")
.setFragment("fragment");
final String result = uribuilder.toString();
Assertions.assertEquals("invalid://", result);
}
}

0 comments on commit 9ca51ab

Please sign in to comment.