Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 53 additions & 12 deletions be/src/io/fs/http_file_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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.
Expand All @@ -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: {}",
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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 "
Expand Down
34 changes: 34 additions & 0 deletions be/src/service/http/http_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include <memory>
#include <ostream>
#include <string_view>

#include "common/cast_set.h"
#include "common/config.h"
Expand Down Expand Up @@ -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 <start>-<end>/<total>", 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();
Expand Down
7 changes: 7 additions & 0 deletions be/src/service/http/http_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <start>-<end>/<total>" (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);
Expand Down
226 changes: 226 additions & 0 deletions be/test/io/fs/http_file_reader_test.cpp
Original file line number Diff line number Diff line change
@@ -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 <gtest/gtest.h>

#include <algorithm>
#include <map>
#include <string>

#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<std::string, std::string> 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<void>(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<std::string, std::string> 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<void>(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<std::string, std::string> 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<std::string, std::string> 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<void>(reader->close());
}

} // namespace doris::io
Loading
Loading