From e99b9c96528a275499c49c2c9ce6df13d2024d6d Mon Sep 17 00:00:00 2001 From: YuMin Kim Date: Thu, 5 Nov 2026 11:07:31 +0900 Subject: [PATCH] utils: follow http to https redirects when reading the QCOW2 virtual size HttpURLConnection never follows redirects that change the protocol, so a direct download QCOW2 template registered with an http:// URL that redirects to https:// failed the virtual size check with "Failed to connect URL". When redirects are enabled, follow them in QCOW2Utils with a limit of 10 hops, resolving relative Location headers against the current URL. Only http and https targets are followed and https is never downgraded to http. Fixes #14506 Assisted-by: Claude Opus 5.5 (Claude Code) --- .../com/cloud/utils/storage/QCOW2Utils.java | 42 +++++- .../cloud/utils/storage/QCOW2UtilsTest.java | 126 +++++++++++++++++- 2 files changed, 163 insertions(+), 5 deletions(-) diff --git a/utils/src/main/java/com/cloud/utils/storage/QCOW2Utils.java b/utils/src/main/java/com/cloud/utils/storage/QCOW2Utils.java index a4a49825d1f6..a0332780ce7f 100644 --- a/utils/src/main/java/com/cloud/utils/storage/QCOW2Utils.java +++ b/utils/src/main/java/com/cloud/utils/storage/QCOW2Utils.java @@ -25,6 +25,7 @@ import java.net.HttpURLConnection; import java.net.URI; import java.net.URISyntaxException; +import java.net.URL; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; @@ -43,6 +44,7 @@ public final class QCOW2Utils { private static final int VIRTUALSIZE_HEADER_LOCATION = 24; private static final int VIRTUALSIZE_HEADER_LENGTH = 8; private static final int MAGIC_HEADER_LENGTH = 4; + private static final int MAX_REDIRECTS = 10; /** * Private constructor -> This utility class cannot be instantiated. @@ -118,10 +120,21 @@ private static long getVirtualSizeFromInputStream(InputStream inputStream) throw public static long getVirtualSizeFromUrl(String urlStr, boolean followRedirects) { HttpURLConnection httpConn = null; try { - URI url = new URI(urlStr); - httpConn = (HttpURLConnection)url.toURL().openConnection(); - httpConn.setRequestProperty(UriUtils.USER_AGENT, HttpClientCloudStackUserAgent.CLOUDSTACK_USER_AGENT); - httpConn.setInstanceFollowRedirects(followRedirects); + URL url = new URI(urlStr).toURL(); + for (int redirects = 0; ; redirects++) { + httpConn = (HttpURLConnection)url.openConnection(); + httpConn.setRequestProperty(UriUtils.USER_AGENT, HttpClientCloudStackUserAgent.CLOUDSTACK_USER_AGENT); + // HttpURLConnection never follows redirects that change the protocol (e.g. http to https), so redirects are followed here + httpConn.setInstanceFollowRedirects(false); + if (!followRedirects || !isRedirect(httpConn.getResponseCode())) { + break; + } + if (redirects >= MAX_REDIRECTS) { + throw new IOException("Too many redirects, giving up after " + MAX_REDIRECTS); + } + url = getRedirectUrl(url, httpConn.getHeaderField("Location")); + httpConn.disconnect(); + } return getVirtualSize(httpConn.getInputStream(), UriUtils.isUrlForCompressedFile(urlStr)); } catch (URISyntaxException e) { LOGGER.warn("Failed to validate for qcow2, malformed URL: " + urlStr + ", error: " + e.getMessage()); @@ -135,4 +148,25 @@ public static long getVirtualSizeFromUrl(String urlStr, boolean followRedirects) } } } + + private static boolean isRedirect(int responseCode) { + return responseCode == HttpURLConnection.HTTP_MOVED_PERM || responseCode == HttpURLConnection.HTTP_MOVED_TEMP + || responseCode == HttpURLConnection.HTTP_SEE_OTHER || responseCode == 307 || responseCode == 308; + } + + /** + * Resolves the Location header of a redirect against the current URL. + * Only http and https targets are followed, and an https URL is never redirected to http. + */ + static URL getRedirectUrl(URL currentUrl, String location) throws IOException { + if (location == null) { + throw new IOException("Redirect from " + currentUrl + " has no Location header"); + } + URL redirectUrl = new URL(currentUrl, location); + String protocol = redirectUrl.getProtocol(); + if (!"https".equals(protocol) && !("http".equals(protocol) && "http".equals(currentUrl.getProtocol()))) { + throw new IOException("Refusing to follow redirect from " + currentUrl + " to " + redirectUrl); + } + return redirectUrl; + } } diff --git a/utils/src/test/java/com/cloud/utils/storage/QCOW2UtilsTest.java b/utils/src/test/java/com/cloud/utils/storage/QCOW2UtilsTest.java index 0595b2f67dbd..10a067174567 100644 --- a/utils/src/test/java/com/cloud/utils/storage/QCOW2UtilsTest.java +++ b/utils/src/test/java/com/cloud/utils/storage/QCOW2UtilsTest.java @@ -19,20 +19,39 @@ package com.cloud.utils.storage; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.URL; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.After; import org.junit.Before; import org.junit.Test; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; + public class QCOW2UtilsTest { InputStream inputStream; + byte[] qcow2Bytes; + + HttpServer server; + final Long virtualSize = 21474836480L; /** @@ -106,7 +125,15 @@ public void setup() { // Snapshots offset byteBuffer.putLong(0L); - inputStream = new ByteArrayInputStream(byteBuffer.array()); + qcow2Bytes = byteBuffer.array(); + inputStream = new ByteArrayInputStream(qcow2Bytes); + } + + @After + public void tearDown() { + if (server != null) { + server.stop(0); + } } @Test @@ -118,4 +145,101 @@ public void getVirtualSizeHeaderLocation() { public void getVirtualSizeTest() throws IOException { assertEquals(virtualSize.longValue(), QCOW2Utils.getVirtualSize(inputStream, false)); } + + @Test + public void getRedirectUrlResolvesRelativeLocation() throws IOException { + URL redirectUrl = QCOW2Utils.getRedirectUrl(new URL("http://example.com/templates/redirect"), "template.qcow2"); + assertEquals("http://example.com/templates/template.qcow2", redirectUrl.toString()); + } + + @Test + public void getRedirectUrlAllowsHttpToHttps() throws IOException { + URL redirectUrl = QCOW2Utils.getRedirectUrl(new URL("http://example.com/template.qcow2"), "https://example.com/template.qcow2"); + assertEquals("https://example.com/template.qcow2", redirectUrl.toString()); + } + + @Test + public void getRedirectUrlRefusesHttpsToHttp() { + assertThrows(IOException.class, () -> QCOW2Utils.getRedirectUrl(new URL("https://example.com/template.qcow2"), "http://example.com/template.qcow2")); + } + + @Test + public void getRedirectUrlRefusesNonHttpProtocol() { + assertThrows(IOException.class, () -> QCOW2Utils.getRedirectUrl(new URL("http://example.com/template.qcow2"), "file:///etc/passwd")); + } + + @Test + public void getRedirectUrlRefusesMissingLocation() { + assertThrows(IOException.class, () -> QCOW2Utils.getRedirectUrl(new URL("http://example.com/template.qcow2"), null)); + } + + @Test + public void getVirtualSizeFromUrlFollowsRelativeRedirect() throws IOException { + String baseUrl = startServer(exchange -> { + if ("/template.qcow2".equals(exchange.getRequestURI().getPath())) { + exchange.sendResponseHeaders(200, qcow2Bytes.length); + exchange.getResponseBody().write(qcow2Bytes); + exchange.close(); + } else { + sendRedirect(exchange, "template.qcow2"); + } + }); + assertEquals(virtualSize.longValue(), QCOW2Utils.getVirtualSizeFromUrl(baseUrl + "/redirect", true)); + } + + @Test + public void getVirtualSizeFromUrlDoesNotFollowRedirectWhenDisabled() throws IOException { + AtomicInteger requests = new AtomicInteger(); + String baseUrl = startServer(exchange -> { + requests.incrementAndGet(); + sendRedirect(exchange, "/template.qcow2"); + }); + assertThrows(IllegalArgumentException.class, () -> QCOW2Utils.getVirtualSizeFromUrl(baseUrl + "/redirect", false)); + assertEquals(1, requests.get()); + } + + @Test + public void getVirtualSizeFromUrlStopsAfterTooManyRedirects() throws IOException { + AtomicInteger requests = new AtomicInteger(); + String baseUrl = startServer(exchange -> { + requests.incrementAndGet(); + sendRedirect(exchange, "/redirect"); + }); + assertThrows(IllegalArgumentException.class, () -> QCOW2Utils.getVirtualSizeFromUrl(baseUrl + "/redirect", true)); + assertEquals(11, requests.get()); + } + + @Test + public void getVirtualSizeFromUrlFollowsHttpToHttpsRedirect() throws Exception { + // HttpURLConnection does not follow http to https redirects by itself, so check that the https target is contacted + try (ServerSocket httpsTarget = new ServerSocket(0, 1, InetAddress.getLoopbackAddress())) { + CountDownLatch httpsConnected = new CountDownLatch(1); + Thread acceptor = new Thread(() -> { + try (Socket socket = httpsTarget.accept()) { + httpsConnected.countDown(); + } catch (IOException e) { + // the target socket was closed without a connection + } + }); + acceptor.start(); + String baseUrl = startServer(exchange -> sendRedirect(exchange, "https://127.0.0.1:" + httpsTarget.getLocalPort() + "/template.qcow2")); + + // the TLS handshake fails as the target is not an https server + assertThrows(IllegalArgumentException.class, () -> QCOW2Utils.getVirtualSizeFromUrl(baseUrl + "/template.qcow2", true)); + assertTrue(httpsConnected.await(10, TimeUnit.SECONDS)); + } + } + + private String startServer(HttpHandler handler) throws IOException { + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/", handler); + server.start(); + return "http://127.0.0.1:" + server.getAddress().getPort(); + } + + private static void sendRedirect(HttpExchange exchange, String location) throws IOException { + exchange.getResponseHeaders().add("Location", location); + exchange.sendResponseHeaders(302, -1); + exchange.close(); + } }