From a83c7cab715313fda446c0b8c4f8a3414139c312 Mon Sep 17 00:00:00 2001 From: Buffden Date: Mon, 6 Jul 2026 15:40:26 -0500 Subject: [PATCH 1/2] reject URLs with unresolvable domains in reachability check --- .../java/com/tinyurl/service/UrlReachabilityCheckerImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tinyurl/src/main/java/com/tinyurl/service/UrlReachabilityCheckerImpl.java b/tinyurl/src/main/java/com/tinyurl/service/UrlReachabilityCheckerImpl.java index b3c97e5..f4c8fb6 100644 --- a/tinyurl/src/main/java/com/tinyurl/service/UrlReachabilityCheckerImpl.java +++ b/tinyurl/src/main/java/com/tinyurl/service/UrlReachabilityCheckerImpl.java @@ -5,6 +5,7 @@ import java.net.Inet6Address; import java.net.InetAddress; import java.net.URI; +import java.net.UnknownHostException; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; @@ -36,6 +37,8 @@ public void check(String url) { } } catch (UrlUnreachableException e) { throw e; + } catch (UnknownHostException e) { + throw new UrlUnreachableException("URL_UNREACHABLE"); } catch (Exception e) { log.warn("Reachability check failed for host {} — failing open: {}", URI.create(url).getHost(), e.getMessage()); } From 41f61d22010c2554c0de4e0b5b2112405180034d Mon Sep 17 00:00:00 2001 From: Buffden Date: Mon, 6 Jul 2026 16:14:10 -0500 Subject: [PATCH 2/2] harden URL reachability validation against invalid and malicious inputs --- .../service/UrlReachabilityCheckerImpl.java | 78 ++++++--- .../com/tinyurl/service/UrlServiceImpl.java | 3 + .../UrlReachabilityCheckerImplTest.java | 154 ++++++++++++++++++ .../tinyurl/service/UrlServiceImplTest.java | 9 + 4 files changed, 218 insertions(+), 26 deletions(-) create mode 100644 tinyurl/src/test/java/com/tinyurl/service/UrlReachabilityCheckerImplTest.java diff --git a/tinyurl/src/main/java/com/tinyurl/service/UrlReachabilityCheckerImpl.java b/tinyurl/src/main/java/com/tinyurl/service/UrlReachabilityCheckerImpl.java index f4c8fb6..ef4e755 100644 --- a/tinyurl/src/main/java/com/tinyurl/service/UrlReachabilityCheckerImpl.java +++ b/tinyurl/src/main/java/com/tinyurl/service/UrlReachabilityCheckerImpl.java @@ -1,7 +1,7 @@ package com.tinyurl.service; import com.tinyurl.exception.UrlUnreachableException; -import java.io.IOException; +import java.net.ConnectException; import java.net.Inet6Address; import java.net.InetAddress; import java.net.URI; @@ -9,6 +9,7 @@ import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; import java.time.Duration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,38 +19,79 @@ public class UrlReachabilityCheckerImpl implements UrlReachabilityChecker { private static final Logger log = LoggerFactory.getLogger(UrlReachabilityCheckerImpl.class); + private static final int MAX_REDIRECTS = 5; - private final HttpClient httpClient = HttpClient.newBuilder() - .followRedirects(HttpClient.Redirect.NEVER) - .connectTimeout(Duration.ofSeconds(5)) - .build(); + private final HttpClient httpClient; + + public UrlReachabilityCheckerImpl() { + this.httpClient = HttpClient.newBuilder() + .followRedirects(HttpClient.Redirect.NEVER) + .connectTimeout(Duration.ofSeconds(5)) + .build(); + } + + // Package-private for testing + UrlReachabilityCheckerImpl(HttpClient httpClient) { + this.httpClient = httpClient; + } @Override public void check(String url) { try { - guardSsrf(url); - int status = sendHead(url); + int status = sendFollowingRedirects(url, "HEAD"); if (status == 405) { - status = sendGet(url); + status = sendFollowingRedirects(url, "GET"); } - if (status == 404 || status == 410) { + if (!isAcceptableStatus(status)) { throw new UrlUnreachableException("URL_UNREACHABLE"); } } catch (UrlUnreachableException e) { throw e; - } catch (UnknownHostException e) { + } catch (UnknownHostException | ConnectException | HttpTimeoutException e) { throw new UrlUnreachableException("URL_UNREACHABLE"); } catch (Exception e) { log.warn("Reachability check failed for host {} — failing open: {}", URI.create(url).getHost(), e.getMessage()); } } + private int sendFollowingRedirects(String url, String method) throws Exception { + for (int i = 0; i <= MAX_REDIRECTS; i++) { + guardSsrf(url); + HttpRequest.Builder builder = HttpRequest.newBuilder(URI.create(url)) + .timeout(Duration.ofSeconds(5)); + if ("HEAD".equals(method)) { + builder.method("HEAD", HttpRequest.BodyPublishers.noBody()); + } else { + builder.GET(); + } + HttpResponse response = httpClient.send(builder.build(), HttpResponse.BodyHandlers.discarding()); + int status = response.statusCode(); + if (status >= 300 && status < 400) { + String location = response.headers().firstValue("location").orElse(null); + if (location != null) { + url = URI.create(url).resolve(location).toString(); + continue; + } + } + return status; + } + throw new UrlUnreachableException("URL_UNREACHABLE"); + } + + private boolean isAcceptableStatus(int status) { + return status >= 200 && status < 300; + } + private void guardSsrf(String url) throws Exception { String host = URI.create(url).getHost(); if (host == null || host.isBlank()) { throw new UrlUnreachableException("URL_UNREACHABLE"); } InetAddress address = InetAddress.getByName(host); + // Reject raw IP address literals (IPv4 like 1.2.3.4, IPv6 like ::1) + if (host.equals(address.getHostAddress())) { + throw new UrlUnreachableException("URL_UNREACHABLE"); + } if (address.isLoopbackAddress() || address.isSiteLocalAddress() || address.isLinkLocalAddress() @@ -66,20 +108,4 @@ private boolean isIpv6UniqueLocal(InetAddress address) { // FC00::/7 — unique local addresses (covers FC00:: and FD00:: ranges) return (address.getAddress()[0] & 0xFE) == 0xFC; } - - private int sendHead(String url) throws IOException, InterruptedException { - HttpRequest request = HttpRequest.newBuilder(URI.create(url)) - .method("HEAD", HttpRequest.BodyPublishers.noBody()) - .timeout(Duration.ofSeconds(5)) - .build(); - return httpClient.send(request, HttpResponse.BodyHandlers.discarding()).statusCode(); - } - - private int sendGet(String url) throws IOException, InterruptedException { - HttpRequest request = HttpRequest.newBuilder(URI.create(url)) - .GET() - .timeout(Duration.ofSeconds(5)) - .build(); - return httpClient.send(request, HttpResponse.BodyHandlers.discarding()).statusCode(); - } } diff --git a/tinyurl/src/main/java/com/tinyurl/service/UrlServiceImpl.java b/tinyurl/src/main/java/com/tinyurl/service/UrlServiceImpl.java index d666262..64afb70 100644 --- a/tinyurl/src/main/java/com/tinyurl/service/UrlServiceImpl.java +++ b/tinyurl/src/main/java/com/tinyurl/service/UrlServiceImpl.java @@ -110,6 +110,9 @@ private void validateUrl(String rawUrl) { if (!uri.isAbsolute() || uri.getHost() == null || uri.getHost().isBlank()) { throw new IllegalArgumentException("INVALID_URL"); } + if (uri.getUserInfo() != null) { + throw new IllegalArgumentException("INVALID_URL"); + } } catch (URISyntaxException | IllegalArgumentException ex) { throw new IllegalArgumentException("INVALID_URL"); } diff --git a/tinyurl/src/test/java/com/tinyurl/service/UrlReachabilityCheckerImplTest.java b/tinyurl/src/test/java/com/tinyurl/service/UrlReachabilityCheckerImplTest.java new file mode 100644 index 0000000..e8c5b65 --- /dev/null +++ b/tinyurl/src/test/java/com/tinyurl/service/UrlReachabilityCheckerImplTest.java @@ -0,0 +1,154 @@ +package com.tinyurl.service; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; + +import com.tinyurl.exception.UrlUnreachableException; +import java.net.ConnectException; +import java.net.http.HttpClient; +import java.net.http.HttpHeaders; +import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class UrlReachabilityCheckerImplTest { + + @Mock + private HttpClient httpClient; + + private UrlReachabilityCheckerImpl checker; + + @BeforeEach + void setUp() { + checker = new UrlReachabilityCheckerImpl(httpClient); + } + + @Test + void shouldRejectUnresolvableDomain() { + assertThrows(UrlUnreachableException.class, + () -> checker.check("https://this-domain-definitely-does-not-exist-xyz123abc.com")); + } + + @Test + void shouldRejectRawIpv4Address() { + assertThrows(UrlUnreachableException.class, + () -> checker.check("https://8.8.8.8")); + } + + @Test + void shouldRejectLoopbackAddress() { + assertThrows(UrlUnreachableException.class, + () -> checker.check("https://127.0.0.1")); + } + + @Test + void shouldRejectPrivateRangeAddress() { + assertThrows(UrlUnreachableException.class, + () -> checker.check("https://192.168.1.1")); + } + + @Test + void shouldAcceptWhenHeadReturns200() throws Exception { + doReturn(response(200)).when(httpClient).send(any(), any()); + assertDoesNotThrow(() -> checker.check("https://example.com")); + } + + @Test + void shouldRejectWhenHeadReturns404() throws Exception { + doReturn(response(404)).when(httpClient).send(any(), any()); + assertThrows(UrlUnreachableException.class, () -> checker.check("https://example.com")); + } + + @Test + void shouldRejectWhenHeadReturns500() throws Exception { + doReturn(response(500)).when(httpClient).send(any(), any()); + assertThrows(UrlUnreachableException.class, () -> checker.check("https://example.com")); + } + + @Test + void shouldFallbackToGetWhen405AndAcceptOn200() throws Exception { + HttpResponse res405 = response(405); + HttpResponse res200 = response(200); + doReturn(res405).doReturn(res200).when(httpClient).send(any(), any()); + assertDoesNotThrow(() -> checker.check("https://example.com")); + } + + @Test + void shouldRejectWhenBothHeadAndGetFail() throws Exception { + HttpResponse res405 = response(405); + HttpResponse res500 = response(500); + doReturn(res405).doReturn(res500).when(httpClient).send(any(), any()); + assertThrows(UrlUnreachableException.class, () -> checker.check("https://example.com")); + } + + + @Test + void shouldRejectOnTimeout() throws Exception { + doThrow(new HttpTimeoutException("timeout")).when(httpClient).send(any(), any()); + assertThrows(UrlUnreachableException.class, () -> checker.check("https://example.com")); + } + + @Test + void shouldRejectOnConnectionRefused() throws Exception { + doThrow(new ConnectException("refused")).when(httpClient).send(any(), any()); + assertThrows(UrlUnreachableException.class, () -> checker.check("https://example.com")); + } + + + @Test + void shouldFollowRedirectAndAcceptOn200() throws Exception { + HttpResponse redirect = responseWithLocation(301, "https://example.com/final"); + HttpResponse ok = response(200); + doReturn(redirect).doReturn(ok).when(httpClient).send(any(), any()); + assertDoesNotThrow(() -> checker.check("https://example.com/original")); + } + + @Test + void shouldRejectWhenRedirectExceedsMaxHops() throws Exception { + doReturn(responseWithLocation(301, "https://example.com/loop")) + .when(httpClient).send(any(), any()); + assertThrows(UrlUnreachableException.class, () -> checker.check("https://example.com/start")); + } + + @Test + void shouldRejectRedirectToRawPublicIp() throws Exception { + doReturn(responseWithLocation(301, "https://8.8.8.8/path")) + .when(httpClient).send(any(), any()); + assertThrows(UrlUnreachableException.class, () -> checker.check("https://example.com")); + } + + @Test + void shouldRejectRedirectToPrivateIp() throws Exception { + doReturn(responseWithLocation(301, "https://192.168.1.1/internal")) + .when(httpClient).send(any(), any()); + assertThrows(UrlUnreachableException.class, () -> checker.check("https://example.com")); + } + + + @SuppressWarnings("unchecked") + private HttpResponse response(int statusCode) { + HttpResponse r = mock(HttpResponse.class); + doReturn(statusCode).when(r).statusCode(); + return r; + } + + @SuppressWarnings("unchecked") + private HttpResponse responseWithLocation(int statusCode, String location) { + HttpResponse r = mock(HttpResponse.class); + doReturn(statusCode).when(r).statusCode(); + doReturn(HttpHeaders.of(Map.of("location", List.of(location)), (a, b) -> true)) + .when(r).headers(); + return r; + } +} diff --git a/tinyurl/src/test/java/com/tinyurl/service/UrlServiceImplTest.java b/tinyurl/src/test/java/com/tinyurl/service/UrlServiceImplTest.java index 7b8ab03..c042145 100644 --- a/tinyurl/src/test/java/com/tinyurl/service/UrlServiceImplTest.java +++ b/tinyurl/src/test/java/com/tinyurl/service/UrlServiceImplTest.java @@ -64,6 +64,15 @@ void shortenUrlShouldRejectNonHttpUrl() { assertEquals("INVALID_URL", ex.getMessage()); } + @Test + void shortenUrlShouldRejectUrlWithCredentials() { + IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> service.shortenUrl(new CreateUrlRequest("https://user:pass@example.com", 30), null, null, null) + ); + assertEquals("INVALID_URL", ex.getMessage()); + } + @Test void shortenUrlShouldRejectZeroExpiry() { IllegalArgumentException ex = assertThrows(