From ab12d564791c90785446566d80d36a9256cd43e8 Mon Sep 17 00:00:00 2001 From: Zhigao Hong Date: Sun, 9 Aug 2026 17:30:00 +0800 Subject: [PATCH 1/2] fix(http): fall back to GET when HEAD is rejected Some servers reject HEAD requests, leaving the HTTP file size unknown. Fall back to a ranged GET (Range: bytes=0-0) and parse the total from the Content-Range header on 206, or Content-Length on 200, so downloads/reads still work. - BE: add HttpClient::get_content_range_total; recover size from Content-Range/Content-Length during range-support detection - FE: add ranged-GET fallback in HttpUtils file-size probing --- be/src/io/fs/http_file_reader.cpp | 65 ++++++-- be/src/service/http/http_client.cpp | 34 ++++ be/src/service/http/http_client.h | 7 + .../doris/httpv2/rest/manager/HttpUtils.java | 149 ++++++++++++++---- 4 files changed, 211 insertions(+), 44 deletions(-) diff --git a/be/src/io/fs/http_file_reader.cpp b/be/src/io/fs/http_file_reader.cpp index dc676e4a9f413e..129486702b8e72 100644 --- a/be/src/io/fs/http_file_reader.cpp +++ b/be/src/io/fs/http_file_reader.cpp @@ -146,6 +146,7 @@ Status HttpFileReader::open(const FileReaderOptions& opts) { } // Step 1: HEAD request to get file metadata (skip for chunk response) + bool range_probed = false; if (_enable_chunk_response) { // Chunk streaming response: size is unknown until the stream completes. // _range_supported is already false (set in constructor). @@ -156,16 +157,32 @@ Status HttpFileReader::open(const FileReaderOptions& opts) { _file_size = 0; LOG(INFO) << "Chunk response mode enabled, skipping HEAD request for " << _url; } else { - // Normal mode: execute HEAD request to get file metadata - RETURN_IF_ERROR(prepare_client(/*set_fail_on_error=*/true)); + // Normal mode: try a HEAD request to get file metadata (size). Some resources + // reject HEAD requests -- most notably presigned object-storage URLs whose + // signature covers the HTTP method, so a URL signed for GET is rejected when + // accessed via HEAD even though the same URL works fine with GET. A HEAD failure + // must not abort opening the file: we fall back to a GET-based Range probe below, + // which mirrors the actual read path and works for such URLs. + RETURN_IF_ERROR(prepare_client(/*set_fail_on_error=*/false)); _client->set_method(HttpMethod::HEAD); - RETURN_IF_ERROR(_client->execute()); - - uint64_t content_length = 0; - RETURN_IF_ERROR(_client->get_content_length(&content_length)); + Status head_status = _client->execute(); + if (head_status.ok() && _client->get_http_status() == 200) { + uint64_t content_length = 0; + RETURN_IF_ERROR(_client->get_content_length(&content_length)); + _file_size = content_length; + _size_known = true; + } else { + LOG(INFO) << "HEAD request failed for " << _url + << " (status: " << (head_status.ok() ? _client->get_http_status() : -1) + << ", err: " << head_status << "), falling back to GET-based probe."; + } - _file_size = content_length; - _size_known = true; + // If HEAD did not yield a size, probe with a ranged GET (the same probe used to + // detect Range support), which also recovers the size from Content-Range/Content-Length. + if (!_size_known) { + RETURN_IF_ERROR(detect_range_support()); + range_probed = true; + } } // Step 2: Check if Range request is disabled by configuration. @@ -176,7 +193,7 @@ Status HttpFileReader::open(const FileReaderOptions& opts) { } else if (!_enable_range_request) { _range_supported = false; LOG(INFO) << "Range requests disabled by configuration for " << _url; - if (_file_size > _max_request_size_bytes) { + if (_size_known && _file_size > _max_request_size_bytes) { return Status::InternalError( "Non-Range mode: file size ({} bytes) exceeds maximum allowed size ({} " "bytes, configured by http.max.request.size.bytes). URL: {}", @@ -185,9 +202,12 @@ Status HttpFileReader::open(const FileReaderOptions& opts) { LOG(INFO) << "Non-Range mode validated for " << _url << ", file size: " << _file_size << " bytes, max allowed: " << _max_request_size_bytes << " bytes"; } else { - // Step 3: Range request is enabled (default), detect Range support - VLOG(1) << "Detecting Range support for URL: " << _url; - RETURN_IF_ERROR(detect_range_support()); + // Step 3: Range request is enabled (default), detect Range support if not already + // done above during the size-probing fallback. + if (!range_probed) { + VLOG(1) << "Detecting Range support for URL: " << _url; + RETURN_IF_ERROR(detect_range_support()); + } // Step 4: Validate Range support detection result if (!_range_supported) { @@ -518,6 +538,18 @@ Status HttpFileReader::detect_range_support() { _range_supported = true; VLOG(1) << "Range support detected (HTTP 206) for " << _url << ", received " << test_buf.size() << " bytes"; + + // Recover the total file size from the Content-Range header (e.g. "bytes 0-1/12345") + // when it was not already obtained from a HEAD request. + if (!_size_known) { + uint64_t total = 0; + if (_client->get_content_range_total(&total).ok()) { + _file_size = total; + _size_known = true; + VLOG(1) << "File size recovered from Content-Range for " << _url << ": " + << _file_size << " bytes"; + } + } } else if (http_status == 200) { // HTTP 200 OK - server does not support Range requests // It returned the full file (or a large portion) @@ -530,6 +562,15 @@ Status HttpFileReader::detect_range_support() { LOG(WARNING) << "Server returned " << received << "+ bytes for Range test, " << "indicating no Range support for " << _url; } + + // Recover the total file size from Content-Length when not already known. + if (!_size_known) { + uint64_t content_length = 0; + if (_client->get_content_length(&content_length).ok()) { + _file_size = content_length; + _size_known = true; + } + } } else { // Unexpected status code LOG(WARNING) << "Unexpected HTTP status " << http_status << " during Range detection for " diff --git a/be/src/service/http/http_client.cpp b/be/src/service/http/http_client.cpp index 7141e6f6012d0a..dd19e46b889bd0 100644 --- a/be/src/service/http/http_client.cpp +++ b/be/src/service/http/http_client.cpp @@ -23,6 +23,7 @@ #include #include +#include #include "common/cast_set.h" #include "common/config.h" @@ -460,6 +461,39 @@ Status HttpClient::get_content_md5(std::string* md5) const { return Status::OK(); } +Status HttpClient::get_content_range_total(uint64_t* total) const { + struct curl_header* header_ptr; + auto code = + curl_easy_header(_curl, HttpHeaders::CONTENT_RANGE, 0, CURLH_HEADER, 0, &header_ptr); + if (code == CURLHE_MISSING || code == CURLHE_NOHEADERS) { + return Status::NotFound("no Content-Range header in response"); + } else if (code != CURLHE_OK) { + return Status::HttpError("failed to get http header {}: {} ({})", + HttpHeaders::CONTENT_RANGE, header_error_msg(code), code); + } + + // Format: "bytes -/", e.g. "bytes 0-0/12345". + // The total may be "*" when the server does not know the full size. + std::string_view value(header_ptr->value); + size_t slash_pos = value.rfind('/'); + if (slash_pos == std::string_view::npos || slash_pos + 1 >= value.size()) { + return Status::NotFound("malformed Content-Range header: {}", header_ptr->value); + } + std::string_view total_view = value.substr(slash_pos + 1); + if (total_view == "*") { + return Status::NotFound("unknown total size in Content-Range header: {}", + header_ptr->value); + } + uint64_t parsed = 0; + auto res = std::from_chars(total_view.data(), total_view.data() + total_view.size(), parsed); + if (res.ec != std::errc() || res.ptr != total_view.data() + total_view.size()) { + return Status::NotFound("invalid total size in Content-Range header: {}", + header_ptr->value); + } + *total = parsed; + return Status::OK(); +} + Status HttpClient::download(const std::string& local_path) { set_method(GET); set_speed_limit(); diff --git a/be/src/service/http/http_client.h b/be/src/service/http/http_client.h index 8c57d8a2e6aa8b..72fc2215ce1222 100644 --- a/be/src/service/http/http_client.h +++ b/be/src/service/http/http_client.h @@ -138,6 +138,13 @@ class HttpClient { // Get the value of the header CONTENT-MD5. The output is empty if no such header exists. Status get_content_md5(std::string* md5) const; + // Parse the total resource size from the "Content-Range" response header. + // The header format is "bytes -/" (e.g. "bytes 0-0/12345"), + // returned together with a 206 Partial Content response. On success *total is + // set to the parsed size. Returns an error if the header is missing or the + // total part is unknown ("*") or malformed. + Status get_content_range_total(uint64_t* total) const; + long get_http_status() const { long code; curl_easy_getinfo(_curl, CURLINFO_RESPONSE_CODE, &code); diff --git a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/HttpUtils.java b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/HttpUtils.java index 43df88ee548506..af2226b8768e15 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/HttpUtils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/manager/HttpUtils.java @@ -47,6 +47,7 @@ import java.net.HttpURLConnection; import java.net.URL; import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -157,64 +158,148 @@ public static String getBody(HttpServletRequest request) throws IOException { } /** - * Get the file size of the HTTP resource by sending a HEAD request. - * This method uses HTTP HEAD request to get the Content-Length header - * without downloading the entire file content. + * Get the file size of the HTTP resource. + * + *

+ * This first tries an HTTP HEAD request to read the Content-Length header without + * downloading the file body. Some resources reject HEAD requests, most notably presigned + * object-storage URLs whose signature covers the HTTP method: a URL signed for GET is + * rejected with 403 when accessed via HEAD, even though the same URL works fine with GET. + * In that case we fall back to a GET request with {@code Range: bytes=0-0}, which mirrors + * the actual read path used later and lets us recover the size from the {@code Content-Range} + * (206) or {@code Content-Length} (200) response header. + * * @param uri the HTTP URI to get file size for * @return the file size in bytes, or -1 if the size cannot be determined - * @throws IOException if there's an error connecting to the HTTP resource + * @throws IOException if there's an error connecting to the HTTP resource * @throws IllegalArgumentException if the URI is null or invalid */ public static long getHttpFileSize(String uri, Map headers) throws IOException { if (uri == null || uri.trim().isEmpty()) { throw new IllegalArgumentException("HTTP URI is null or empty"); } + Map safeHeaders = headers != null + ? headers + : Collections.emptyMap(); + try { + Long size = tryGetFileSizeWithHead(uri, safeHeaders); + if (size != null) { + return size; + } + } catch (IOException e) { + LOG.warn("HEAD request failed for URI: {}, falling back to GET with Range. {}", uri, e.getMessage()); + } + + try { + Long size = tryGetFileSizeWithGetRange(uri, safeHeaders); + if (size != null) { + return size; + } + return -1; + } catch (IOException e) { + LOG.warn("Failed to get file size for URI: {}", uri, e); + throw new IOException("Failed to get file size for URI: " + uri + ". " + Util.getRootCauseMessage(e), e); + } + } + + /** + * Try to get the file size via a HEAD request. + * + * @return the file size, or null if the response was OK but had no Content-Length header + * @throws IOException if the connection fails or the response code is not 2xx + */ + private static Long tryGetFileSizeWithHead(String uri, Map headers) throws IOException { HttpURLConnection connection = null; try { URL url = new URL(uri); connection = (HttpURLConnection) url.openConnection(); - - // Use HEAD request to get headers without downloading content connection.setRequestMethod("HEAD"); - connection.setConnectTimeout(10000); // 10 seconds connection timeout - connection.setReadTimeout(30000); // 30 seconds read timeout - - // Set common headers - connection.setRequestProperty("User-Agent", "Doris-HttpUtils/1.0"); - connection.setRequestProperty("Accept", "*/*"); - for (Map.Entry entry : headers.entrySet()) { - connection.setRequestProperty(entry.getKey(), entry.getValue()); + configureFileSizeRequest(connection, headers); + + connection.connect(); + int responseCode = connection.getResponseCode(); + if (responseCode != HttpURLConnection.HTTP_OK) { + throw new IOException("HEAD request failed with response code: " + responseCode + ", message: " + + connection.getResponseMessage()); + } + return parseContentLength(connection.getHeaderField("Content-Length")); + } finally { + if (connection != null) { + connection.disconnect(); } + } + } + + /** + * Try to get the file size via a GET request with {@code Range: bytes=0-0}, used as a + * fallback when the HEAD request is rejected by the server. + * + * @return the file size, or null if it cannot be determined from the response headers + * @throws IOException if the connection fails or the response code is not 2xx/206 + */ + private static Long tryGetFileSizeWithGetRange(String uri, Map headers) throws IOException { + HttpURLConnection connection = null; + try { + URL url = new URL(uri); + connection = (HttpURLConnection) url.openConnection(); + connection.setRequestMethod("GET"); + connection.setRequestProperty("Range", "bytes=0-0"); + configureFileSizeRequest(connection, headers); - // Connect and get response connection.connect(); int responseCode = connection.getResponseCode(); + if (responseCode != HttpURLConnection.HTTP_PARTIAL && responseCode != HttpURLConnection.HTTP_OK) { + throw new IOException("GET request with Range failed with response code: " + responseCode + + ", message: " + connection.getResponseMessage()); + } - if (responseCode == HttpURLConnection.HTTP_OK) { - // Try to get Content-Length header - String contentLengthStr = connection.getHeaderField("Content-Length"); - if (contentLengthStr != null && !contentLengthStr.trim().isEmpty()) { - try { - return Long.parseLong(contentLengthStr.trim()); - } catch (NumberFormatException e) { - throw new IOException("Invalid Content-Length header: " + contentLengthStr, e); + if (responseCode == HttpURLConnection.HTTP_PARTIAL) { + // Format: "bytes 0-0/12345" or "bytes 0-0/*" + String contentRange = connection.getHeaderField("Content-Range"); + if (contentRange != null) { + int slashIdx = contentRange.lastIndexOf('/'); + if (slashIdx >= 0 && slashIdx < contentRange.length() - 1) { + String totalStr = contentRange.substring(slashIdx + 1).trim(); + if (!totalStr.equals("*")) { + try { + return Long.parseLong(totalStr); + } catch (NumberFormatException e) { + LOG.warn("Invalid Content-Range total for URI: {}: {}", uri, contentRange); + } + } } - } else { - // Content-Length header not available - return -1; } - } else { - throw new IOException("HTTP request failed with response code: " + responseCode - + ", message: " + connection.getResponseMessage()); + return null; } - } catch (IOException e) { - LOG.warn("Failed to get file size for URI: {}", uri, e); - throw new IOException("Failed to get file size for URI: " + uri + ". " + Util.getRootCauseMessage(e), e); + // HTTP 200: server ignored Range and returned the full content; Content-Length + // (if present) is the full file size. + return parseContentLength(connection.getHeaderField("Content-Length")); } finally { if (connection != null) { connection.disconnect(); } } } + + private static void configureFileSizeRequest(HttpURLConnection connection, Map headers) { + connection.setConnectTimeout(10000); // 10 seconds connection timeout + connection.setReadTimeout(30000); // 30 seconds read timeout + connection.setRequestProperty("User-Agent", "Doris-HttpUtils/1.0"); + connection.setRequestProperty("Accept", "*/*"); + for (Map.Entry entry : headers.entrySet()) { + connection.setRequestProperty(entry.getKey(), entry.getValue()); + } + } + + private static Long parseContentLength(String contentLengthStr) throws IOException { + if (contentLengthStr == null || contentLengthStr.trim().isEmpty()) { + return null; + } + try { + return Long.parseLong(contentLengthStr.trim()); + } catch (NumberFormatException e) { + throw new IOException("Invalid Content-Length header: " + contentLengthStr, e); + } + } } From 34fcb1d38e605fed100cfba4963053e125fc5c4c Mon Sep 17 00:00:00 2001 From: Zhigao Hong Date: Sun, 9 Aug 2026 21:50:05 +0800 Subject: [PATCH 2/2] test(http): cover HEAD-forbidden fallback for file size probe Add BE/FE unit tests and a regression case simulating presigned URLs whose signature only allows GET. HEAD returns 403, so file-size detection must fall back to a GET-based probe instead of failing. --- be/test/io/fs/http_file_reader_test.cpp | 226 ++++++++++++++++++ .../httpv2/rest/manager/HttpUtilsTest.java | 123 ++++++++++ .../external_table_p0/tvf/test_http_tvf.out | 3 + .../tvf/test_http_tvf.groovy | 26 ++ 4 files changed, 378 insertions(+) create mode 100644 be/test/io/fs/http_file_reader_test.cpp diff --git a/be/test/io/fs/http_file_reader_test.cpp b/be/test/io/fs/http_file_reader_test.cpp new file mode 100644 index 00000000000000..8e7232fdea915c --- /dev/null +++ b/be/test/io/fs/http_file_reader_test.cpp @@ -0,0 +1,226 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "io/fs/http_file_reader.h" + +#include + +#include +#include +#include + +#include "io/fs/file_reader.h" +#include "service/http/ev_http_server.h" +#include "service/http/http_channel.h" +#include "service/http/http_handler.h" +#include "service/http/http_headers.h" +#include "service/http/http_request.h" +#include "util/slice.h" + +namespace doris::io { + +// Full file content served by the test handlers. +static const std::string kFileContent = "0123456789abcdefghij"; // 20 bytes + +// Simulates a presigned URL that is signed for GET only: +// - HEAD is rejected with 403 (as object stores do when the signature covers the method). +// - GET (with or without Range) succeeds. Range requests get 206 + Content-Range. +// This reproduces the reported bug where the HEAD-based probe failed with 403. +class PresignedGetOnlyHandler : public HttpHandler { +public: + void handle(HttpRequest* req) override { + if (req->method() == HttpMethod::HEAD) { + HttpChannel::send_reply(req, HttpStatus::FORBIDDEN, "forbidden"); + return; + } + serve_get(req); + } + +private: + void serve_get(HttpRequest* req) { + const std::string& range = req->header(HttpHeaders::RANGE); + if (range.empty()) { + HttpChannel::send_reply(req, HttpStatus::OK, kFileContent); + return; + } + // Parse "bytes=start-end". + size_t eq = range.find('='); + size_t dash = range.find('-', eq + 1); + size_t start = std::stoul(range.substr(eq + 1, dash - eq - 1)); + size_t end; + std::string end_str = range.substr(dash + 1); + if (end_str.empty()) { + end = kFileContent.size() - 1; + } else { + end = std::stoul(end_str); + } + end = std::min(end, kFileContent.size() - 1); + std::string body = kFileContent.substr(start, end - start + 1); + + std::string content_range = "bytes " + std::to_string(start) + "-" + std::to_string(end) + + "/" + std::to_string(kFileContent.size()); + req->add_output_header(HttpHeaders::CONTENT_RANGE, content_range.c_str()); + HttpChannel::send_reply(req, HttpStatus::PARTIAL_CONTENT, body); + } +}; + +// A resource that is genuinely forbidden for all methods (HEAD and GET both fail). +class AlwaysForbiddenHandler : public HttpHandler { +public: + void handle(HttpRequest* req) override { + HttpChannel::send_reply(req, HttpStatus::FORBIDDEN, "forbidden"); + } +}; + +// A well-behaved resource: HEAD returns 200 + Content-Length, and ranged GET returns +// 206 + Content-Range. Exercises the normal (non-fallback) open() path. +class NormalHandler : public HttpHandler { +public: + void handle(HttpRequest* req) override { + if (req->method() == HttpMethod::HEAD) { + req->add_output_header(HttpHeaders::CONTENT_LENGTH, + std::to_string(kFileContent.size()).c_str()); + HttpChannel::send_reply(req, HttpStatus::OK, ""); + return; + } + const std::string& range = req->header(HttpHeaders::RANGE); + if (range.empty()) { + HttpChannel::send_reply(req, HttpStatus::OK, kFileContent); + return; + } + size_t eq = range.find('='); + size_t dash = range.find('-', eq + 1); + size_t start = std::stoul(range.substr(eq + 1, dash - eq - 1)); + std::string end_str = range.substr(dash + 1); + size_t end = end_str.empty() ? kFileContent.size() - 1 : std::stoul(end_str); + end = std::min(end, kFileContent.size() - 1); + std::string body = kFileContent.substr(start, end - start + 1); + std::string content_range = "bytes " + std::to_string(start) + "-" + std::to_string(end) + + "/" + std::to_string(kFileContent.size()); + req->add_output_header(HttpHeaders::CONTENT_RANGE, content_range.c_str()); + HttpChannel::send_reply(req, HttpStatus::PARTIAL_CONTENT, body); + } +}; + +class HttpFileReaderTest : public testing::Test { +public: + static void SetUpTestCase() { + s_server = new EvHttpServer(0); + s_server->register_handler(HttpMethod::HEAD, "/presigned", &s_presigned_handler); + s_server->register_handler(HttpMethod::GET, "/presigned", &s_presigned_handler); + s_server->register_handler(HttpMethod::HEAD, "/forbidden", &s_forbidden_handler); + s_server->register_handler(HttpMethod::GET, "/forbidden", &s_forbidden_handler); + s_server->register_handler(HttpMethod::HEAD, "/normal", &s_normal_handler); + s_server->register_handler(HttpMethod::GET, "/normal", &s_normal_handler); + s_server->start(); + s_port = s_server->get_real_port(); + ASSERT_NE(0, s_port); + s_host = "http://127.0.0.1:" + std::to_string(s_port); + } + + static void TearDownTestCase() { delete s_server; } + + static EvHttpServer* s_server; + static int s_port; + static std::string s_host; + static PresignedGetOnlyHandler s_presigned_handler; + static AlwaysForbiddenHandler s_forbidden_handler; + static NormalHandler s_normal_handler; +}; + +EvHttpServer* HttpFileReaderTest::s_server = nullptr; +int HttpFileReaderTest::s_port = 0; +std::string HttpFileReaderTest::s_host; +PresignedGetOnlyHandler HttpFileReaderTest::s_presigned_handler; +AlwaysForbiddenHandler HttpFileReaderTest::s_forbidden_handler; +NormalHandler HttpFileReaderTest::s_normal_handler; + +// The normal path: HEAD succeeds and returns the size via Content-Length; reads then work +// over Range requests without triggering any fallback. +TEST_F(HttpFileReaderTest, open_uses_head_size_when_head_succeeds) { + std::map props; + FileReaderOptions opts; + auto res = HttpFileReader::create(s_host + "/normal", props, opts, nullptr); + ASSERT_TRUE(res.has_value()) << res.error(); + auto reader = res.value(); + + EXPECT_EQ(kFileContent.size(), reader->size()); + + std::string buf; + buf.resize(kFileContent.size()); + size_t bytes_read = 0; + auto st = reader->read_at(0, Slice(buf.data(), buf.size()), &bytes_read); + ASSERT_TRUE(st.ok()) << st; + EXPECT_EQ(kFileContent.size(), bytes_read); + EXPECT_EQ(kFileContent, buf); + + static_cast(reader->close()); +} + +// The core regression: a URL whose HEAD returns 403 must still open and read +// via the GET-based fallback, recovering the size from Content-Range. +TEST_F(HttpFileReaderTest, open_falls_back_to_get_when_head_forbidden) { + std::map props; + FileReaderOptions opts; + auto res = HttpFileReader::create(s_host + "/presigned", props, opts, nullptr); + ASSERT_TRUE(res.has_value()) << res.error(); + auto reader = res.value(); + + // Size recovered from the Content-Range header of the ranged GET probe. + EXPECT_EQ(kFileContent.size(), reader->size()); + + // Read the whole file back and verify contents. + std::string buf; + buf.resize(kFileContent.size()); + size_t bytes_read = 0; + auto st = reader->read_at(0, Slice(buf.data(), buf.size()), &bytes_read); + ASSERT_TRUE(st.ok()) << st; + EXPECT_EQ(kFileContent.size(), bytes_read); + EXPECT_EQ(kFileContent, buf); + + static_cast(reader->close()); +} + +// A resource forbidden for every method (GET included) must fail to open, +// rather than being silently swallowed. +TEST_F(HttpFileReaderTest, open_fails_when_all_methods_forbidden) { + std::map props; + FileReaderOptions opts; + auto res = HttpFileReader::create(s_host + "/forbidden", props, opts, nullptr); + EXPECT_FALSE(res.has_value()); +} + +// Reading a sub-range must also work through the fallback path. +TEST_F(HttpFileReaderTest, read_middle_range_after_head_forbidden) { + std::map props; + FileReaderOptions opts; + auto res = HttpFileReader::create(s_host + "/presigned", props, opts, nullptr); + ASSERT_TRUE(res.has_value()) << res.error(); + auto reader = res.value(); + + std::string buf; + buf.resize(5); + size_t bytes_read = 0; + auto st = reader->read_at(10, Slice(buf.data(), buf.size()), &bytes_read); + ASSERT_TRUE(st.ok()) << st; + EXPECT_EQ(5, bytes_read); + EXPECT_EQ(kFileContent.substr(10, 5), buf); + + static_cast(reader->close()); +} + +} // namespace doris::io diff --git a/fe/fe-core/src/test/java/org/apache/doris/httpv2/rest/manager/HttpUtilsTest.java b/fe/fe-core/src/test/java/org/apache/doris/httpv2/rest/manager/HttpUtilsTest.java index 2efaf4ca7bad16..ee50ac9eaf163b 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/httpv2/rest/manager/HttpUtilsTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/httpv2/rest/manager/HttpUtilsTest.java @@ -20,13 +20,20 @@ import org.apache.doris.common.Config; import org.apache.doris.common.util.InternalHttpsUtils; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpHandler; +import com.sun.net.httpserver.HttpServer; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import java.io.IOException; +import java.io.OutputStream; import java.lang.reflect.Field; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.util.Collections; public class HttpUtilsTest { @@ -36,6 +43,7 @@ public class HttpUtilsTest { private boolean originalEnableHttps; private String originalKeyStorePath; + private HttpServer httpServer; @Before public void setUp() throws Exception { @@ -49,6 +57,21 @@ public void tearDown() throws Exception { Config.enable_https = originalEnableHttps; Config.key_store_path = originalKeyStorePath; resetCachedSslContext(); + if (httpServer != null) { + httpServer.stop(0); + } + } + + private String startServer(HttpHandler handler) throws IOException { + httpServer = HttpServer.create(new InetSocketAddress(0), 0); + httpServer.createContext("/file.txt", handler); + httpServer.start(); + return "http://127.0.0.1:" + httpServer.getAddress().getPort() + "/file.txt"; + } + + private static void sendEmpty(HttpExchange exchange, int code) throws IOException { + exchange.sendResponseHeaders(code, -1); + exchange.close(); } private void resetCachedSslContext() throws Exception { @@ -91,4 +114,104 @@ public void testExecuteRequestUsesHttpsClientForHttpsUrl() throws Exception { + e.getMessage()); } } + + @Test + public void testHeadSuccess() throws IOException { + final long totalSize = 12345L; + String url = startServer(exchange -> { + Assert.assertEquals("HEAD", exchange.getRequestMethod()); + exchange.getResponseHeaders().add("Content-Length", String.valueOf(totalSize)); + exchange.sendResponseHeaders(200, -1); + exchange.close(); + }); + + long size = HttpUtils.getHttpFileSize(url, Collections.emptyMap()); + Assert.assertEquals(totalSize, size); + } + + @Test + public void testHeadForbiddenGetRangeReturns206() throws IOException { + byte[] data = "abcdefghij".getBytes(StandardCharsets.UTF_8); + String url = startServer(exchange -> { + if ("HEAD".equals(exchange.getRequestMethod())) { + sendEmpty(exchange, 403); + return; + } + Assert.assertEquals("bytes=0-0", exchange.getRequestHeaders().getFirst("Range")); + exchange.getResponseHeaders().add("Content-Range", "bytes 0-0/" + data.length); + exchange.sendResponseHeaders(206, 1); + try (OutputStream os = exchange.getResponseBody()) { + os.write(data, 0, 1); + } + }); + + long size = HttpUtils.getHttpFileSize(url, Collections.emptyMap()); + Assert.assertEquals(data.length, size); + } + + @Test + public void testHeadForbiddenGetRangeIgnoredReturns200() throws IOException { + byte[] data = "hello world, this is a test file".getBytes(StandardCharsets.UTF_8); + String url = startServer(exchange -> { + if ("HEAD".equals(exchange.getRequestMethod())) { + sendEmpty(exchange, 403); + return; + } + exchange.getResponseHeaders().add("Content-Length", String.valueOf(data.length)); + exchange.sendResponseHeaders(200, data.length); + try (OutputStream os = exchange.getResponseBody()) { + os.write(data); + } + }); + + long size = HttpUtils.getHttpFileSize(url, Collections.emptyMap()); + Assert.assertEquals(data.length, size); + } + + @Test + public void testHeadAndGetBothForbidden() throws IOException { + String url = startServer(exchange -> sendEmpty(exchange, 403)); + + try { + HttpUtils.getHttpFileSize(url, Collections.emptyMap()); + Assert.fail("Expected IOException"); + } catch (IOException e) { + Assert.assertTrue(e.getMessage().contains("Failed to get file size")); + } + } + + @Test + public void testHeadOkNoContentLengthReturnsUnknown() throws IOException { + String url = startServer(exchange -> { + if ("HEAD".equals(exchange.getRequestMethod())) { + exchange.sendResponseHeaders(200, -1); + exchange.close(); + return; + } + exchange.getResponseHeaders().add("Content-Range", "bytes 0-0/*"); + exchange.sendResponseHeaders(206, 1); + try (OutputStream os = exchange.getResponseBody()) { + os.write(new byte[] { 0 }); + } + }); + + long size = HttpUtils.getHttpFileSize(url, Collections.emptyMap()); + Assert.assertEquals(-1, size); + } + + @Test + public void testNullOrEmptyUriRejected() { + try { + HttpUtils.getHttpFileSize(null, Collections.emptyMap()); + Assert.fail("Expected IllegalArgumentException for null uri"); + } catch (IllegalArgumentException | IOException e) { + Assert.assertTrue(e instanceof IllegalArgumentException); + } + try { + HttpUtils.getHttpFileSize(" ", Collections.emptyMap()); + Assert.fail("Expected IllegalArgumentException for blank uri"); + } catch (IllegalArgumentException | IOException e) { + Assert.assertTrue(e instanceof IllegalArgumentException); + } + } } diff --git a/regression-test/data/external_table_p0/tvf/test_http_tvf.out b/regression-test/data/external_table_p0/tvf/test_http_tvf.out index ad2450d8bb7586..4ea036dee18c52 100644 --- a/regression-test/data/external_table_p0/tvf/test_http_tvf.out +++ b/regression-test/data/external_table_p0/tvf/test_http_tvf.out @@ -177,3 +177,6 @@ c1 text Yes false \N NONE -- !sql21 -- !!! Spoiler alert!!!

The point is, though, that I didn't think this film had an ending TO spoil... I only started watching it in the middle, after Matt had gotten into Sarah's body, but then I became fascinated by the bizarreness of the plot, even for a Channel 5 movie... and couldn't possibly see how Matt wld end up happy. What about his fiancee? At one stage looked like he was gonna get with his best friend, surely icky and wrong... and then the whole 'oggi oggi oggi' thing does NOT WORK as a touching buddy-buddy catchphrase, tis just ridiculous... so was going 'surely he can't just come back to life? and yet how can he live as a woman?' and then the film just got over that by ending and not explaining anything at all!!!!! What's that about??? I was so cross, wasted a whole hour of my life for no reason at all!!! :) but was one of the funniest films I've ever seen, so, swings and roundabouts 0 +-- !sql22 -- +2500 + diff --git a/regression-test/suites/external_table_p0/tvf/test_http_tvf.groovy b/regression-test/suites/external_table_p0/tvf/test_http_tvf.groovy index ad0ffffc853e6d..bf6c146cb26720 100644 --- a/regression-test/suites/external_table_p0/tvf/test_http_tvf.groovy +++ b/regression-test/suites/external_table_p0/tvf/test_http_tvf.groovy @@ -77,6 +77,20 @@ suite("test_http_tvf", "p0") { def requestPath = URLDecoder.decode(exchange.requestURI.path, "UTF-8") def relativePath = requestPath.startsWith("/") ? requestPath.substring(1) : requestPath + + // Simulate a presigned URL whose signature only covers GET: HEAD is rejected with 403, + // but GET (with or without Range) succeeds. FE/BE must fall back to a GET-based probe + // to determine the file size instead of failing the query outright. + def headForbiddenPrefix = "head_forbidden/" // prefix that rejects HEAD with 403 + if (relativePath.startsWith(headForbiddenPrefix)) { + if ("HEAD".equalsIgnoreCase(exchange.requestMethod)) { + exchange.sendResponseHeaders(403, -1) + exchange.close() + return + } + relativePath = relativePath.substring(headForbiddenPrefix.length()) + } + def filePath = dataRoot.resolve(relativePath).normalize() if (!filePath.startsWith(dataRoot)) { writeResponse(exchange, 403, "Forbidden".getBytes("UTF-8")) @@ -385,6 +399,18 @@ suite("test_http_tvf", "p0") { "format" = "parquet" ) order by text limit 1; """ + + // Simulate a presigned URL whose signature only covers GET: HEAD is rejected with 403, + // but GET (with or without Range) succeeds. FE/BE must fall back to a GET-based probe + // to determine the file size instead of failing the query outright. + qt_sql22 """ + SELECT count(*) + FROM http( + "uri" = "${httpUrl("head_forbidden/load_p0/http_stream/all_types.csv")}", + "format" = "csv", + "column_separator" = "," + ); + """ } finally { httpServer.stop(0) }