Skip to content

Commit 1b6c387

Browse files
prmoore77prmoore77
committed
Bugfix for header sort order for S3 Requester pays mode.
1 parent e9bb991 commit 1b6c387

File tree

1 file changed

+22
-125
lines changed

1 file changed

+22
-125
lines changed

‎extension/httpfs/s3fs.cpp‎

Lines changed: 22 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -74,30 +74,30 @@ static HTTPHeaders create_s3_header(string url, string query, string host, strin
7474
signed_headers += "content-type;";
7575
}
7676
signed_headers += "host;x-amz-content-sha256;x-amz-date";
77+
if (use_requester_pays) {
78+
signed_headers += ";x-amz-request-payer";
79+
}
7780
if (auth_params.session_token.length() > 0) {
7881
signed_headers += ";x-amz-security-token";
7982
}
8083
if (use_sse_kms) {
8184
signed_headers += ";x-amz-server-side-encryption;x-amz-server-side-encryption-aws-kms-key-id";
8285
}
83-
if (use_requester_pays) {
84-
signed_headers += ";x-amz-request-payer";
85-
}
8686
auto canonical_request = method + "\n" + S3FileSystem::UrlEncode(url) + "\n" + query;
8787
if (content_type.length() > 0) {
8888
canonical_request += "\ncontent-type:" + content_type;
8989
}
9090
canonical_request += "\nhost:" + host + "\nx-amz-content-sha256:" + payload_hash + "\nx-amz-date:" + datetime_now;
91+
if (use_requester_pays) {
92+
canonical_request += "\nx-amz-request-payer:requester";
93+
}
9194
if (auth_params.session_token.length() > 0) {
9295
canonical_request += "\nx-amz-security-token:" + auth_params.session_token;
9396
}
9497
if (use_sse_kms) {
9598
canonical_request += "\nx-amz-server-side-encryption:aws:kms";
9699
canonical_request += "\nx-amz-server-side-encryption-aws-kms-key-id:" + auth_params.kms_key_id;
97100
}
98-
if (use_requester_pays) {
99-
canonical_request += "\nx-amz-request-payer:requester";
100-
}
101101

102102
canonical_request += "\n\n" + signed_headers + "\n" + payload_hash;
103103
sha256(canonical_request.c_str(), canonical_request.length(), canonical_request_hash);
@@ -131,11 +131,6 @@ string S3FileSystem::UrlEncode(const string &input, bool encode_slash) {
131131
return StringUtil::URLEncode(input, encode_slash);
132132
}
133133

134-
static bool IsGCSRequest(const string &url) {
135-
return StringUtil::StartsWith(url, "gcs://") ||
136-
StringUtil::StartsWith(url, "gs://");
137-
}
138-
139134
void AWSEnvironmentCredentialsProvider::SetExtensionOptionValue(string key, const char *env_var_name) {
140135
char *evar;
141136

@@ -214,8 +209,6 @@ S3AuthParams S3AuthParams::ReadFrom(optional_ptr<FileOpener> opener, FileOpenerI
214209
if (result.url_style.empty() || url_style_result.GetScope() != SettingScope::SECRET) {
215210
result.url_style = "path";
216211
}
217-
// Read bearer token for GCS
218-
secret_reader.TryGetSecretKey("bearer_token", result.oauth2_bearer_token);
219212
}
220213

221214
if (!result.region.empty() && (result.endpoint.empty() || result.endpoint == "s3.amazonaws.com")) {
@@ -242,13 +235,9 @@ unique_ptr<KeyValueSecret> CreateSecret(vector<string> &prefix_paths_p, string &
242235
return_value->secret_map["kms_key_id"] = params.kms_key_id;
243236
return_value->secret_map["s3_url_compatibility_mode"] = params.s3_url_compatibility_mode;
244237
return_value->secret_map["requester_pays"] = params.requester_pays;
245-
return_value->secret_map["bearer_token"] = params.oauth2_bearer_token;
246238

247239
//! Set redact keys
248240
return_value->redact_keys = {"secret", "session_token"};
249-
if (!params.oauth2_bearer_token.empty()) {
250-
return_value->redact_keys.insert("bearer_token");
251-
}
252241

253242
return return_value;
254243
}
@@ -683,19 +672,9 @@ unique_ptr<HTTPResponse> S3FileSystem::PostRequest(FileHandle &handle, string ur
683672
auto auth_params = handle.Cast<S3FileHandle>().auth_params;
684673
auto parsed_s3_url = S3UrlParse(url, auth_params);
685674
string http_url = parsed_s3_url.GetHTTPUrl(auth_params, http_params);
686-
687-
HTTPHeaders headers;
688-
if (IsGCSRequest(url) && !auth_params.oauth2_bearer_token.empty()) {
689-
// Use bearer token for GCS
690-
headers["Authorization"] = "Bearer " + auth_params.oauth2_bearer_token;
691-
headers["Host"] = parsed_s3_url.host;
692-
headers["Content-Type"] = "application/octet-stream";
693-
} else {
694-
// Use existing S3 authentication
695-
auto payload_hash = GetPayloadHash(buffer_in, buffer_in_len);
696-
headers = create_s3_header(parsed_s3_url.path, http_params, parsed_s3_url.host, "s3", "POST", auth_params, "",
697-
"", payload_hash, "application/octet-stream");
698-
}
675+
auto payload_hash = GetPayloadHash(buffer_in, buffer_in_len);
676+
auto headers = create_s3_header(parsed_s3_url.path, http_params, parsed_s3_url.host, "s3", "POST", auth_params, "",
677+
"", payload_hash, "application/octet-stream");
699678

700679
return HTTPFileSystem::PostRequest(handle, http_url, headers, result, buffer_in, buffer_in_len);
701680
}
@@ -706,58 +685,28 @@ unique_ptr<HTTPResponse> S3FileSystem::PutRequest(FileHandle &handle, string url
706685
auto parsed_s3_url = S3UrlParse(url, auth_params);
707686
string http_url = parsed_s3_url.GetHTTPUrl(auth_params, http_params);
708687
auto content_type = "application/octet-stream";
709-
710-
HTTPHeaders headers;
711-
if (IsGCSRequest(url) && !auth_params.oauth2_bearer_token.empty()) {
712-
// Use bearer token for GCS
713-
headers["Authorization"] = "Bearer " + auth_params.oauth2_bearer_token;
714-
headers["Host"] = parsed_s3_url.host;
715-
headers["Content-Type"] = content_type;
716-
} else {
717-
// Use existing S3 authentication
718-
auto payload_hash = GetPayloadHash(buffer_in, buffer_in_len);
719-
headers = create_s3_header(parsed_s3_url.path, http_params, parsed_s3_url.host, "s3", "PUT", auth_params, "",
720-
"", payload_hash, content_type);
721-
}
722-
688+
auto payload_hash = GetPayloadHash(buffer_in, buffer_in_len);
689+
690+
auto headers = create_s3_header(parsed_s3_url.path, http_params, parsed_s3_url.host, "s3", "PUT", auth_params, "",
691+
"", payload_hash, content_type);
723692
return HTTPFileSystem::PutRequest(handle, http_url, headers, buffer_in, buffer_in_len);
724693
}
725694

726695
unique_ptr<HTTPResponse> S3FileSystem::HeadRequest(FileHandle &handle, string s3_url, HTTPHeaders header_map) {
727696
auto auth_params = handle.Cast<S3FileHandle>().auth_params;
728697
auto parsed_s3_url = S3UrlParse(s3_url, auth_params);
729698
string http_url = parsed_s3_url.GetHTTPUrl(auth_params);
730-
731-
HTTPHeaders headers;
732-
if (IsGCSRequest(s3_url) && !auth_params.oauth2_bearer_token.empty()) {
733-
// Use bearer token for GCS
734-
headers["Authorization"] = "Bearer " + auth_params.oauth2_bearer_token;
735-
headers["Host"] = parsed_s3_url.host;
736-
} else {
737-
// Use existing S3 authentication
738-
headers = create_s3_header(parsed_s3_url.path, "", parsed_s3_url.host,
739-
"s3", "HEAD", auth_params, "", "", "", "");
740-
}
741-
699+
auto headers =
700+
create_s3_header(parsed_s3_url.path, "", parsed_s3_url.host, "s3", "HEAD", auth_params, "", "", "", "");
742701
return HTTPFileSystem::HeadRequest(handle, http_url, headers);
743702
}
744703

745704
unique_ptr<HTTPResponse> S3FileSystem::GetRequest(FileHandle &handle, string s3_url, HTTPHeaders header_map) {
746705
auto auth_params = handle.Cast<S3FileHandle>().auth_params;
747706
auto parsed_s3_url = S3UrlParse(s3_url, auth_params);
748707
string http_url = parsed_s3_url.GetHTTPUrl(auth_params);
749-
750-
HTTPHeaders headers;
751-
if (IsGCSRequest(s3_url) && !auth_params.oauth2_bearer_token.empty()) {
752-
// Use bearer token for GCS
753-
headers["Authorization"] = "Bearer " + auth_params.oauth2_bearer_token;
754-
headers["Host"] = parsed_s3_url.host;
755-
} else {
756-
// Use existing S3 authentication
757-
headers = create_s3_header(parsed_s3_url.path, "", parsed_s3_url.host,
758-
"s3", "GET", auth_params, "", "", "", "");
759-
}
760-
708+
auto headers =
709+
create_s3_header(parsed_s3_url.path, "", parsed_s3_url.host, "s3", "GET", auth_params, "", "", "", "");
761710
return HTTPFileSystem::GetRequest(handle, http_url, headers);
762711
}
763712

@@ -766,37 +715,17 @@ unique_ptr<HTTPResponse> S3FileSystem::GetRangeRequest(FileHandle &handle, strin
766715
auto auth_params = handle.Cast<S3FileHandle>().auth_params;
767716
auto parsed_s3_url = S3UrlParse(s3_url, auth_params);
768717
string http_url = parsed_s3_url.GetHTTPUrl(auth_params);
769-
770-
HTTPHeaders headers;
771-
if (IsGCSRequest(s3_url) && !auth_params.oauth2_bearer_token.empty()) {
772-
// Use bearer token for GCS
773-
headers["Authorization"] = "Bearer " + auth_params.oauth2_bearer_token;
774-
headers["Host"] = parsed_s3_url.host;
775-
} else {
776-
// Use existing S3 authentication
777-
headers = create_s3_header(parsed_s3_url.path, "", parsed_s3_url.host,
778-
"s3", "GET", auth_params, "", "", "", "");
779-
}
780-
718+
auto headers =
719+
create_s3_header(parsed_s3_url.path, "", parsed_s3_url.host, "s3", "GET", auth_params, "", "", "", "");
781720
return HTTPFileSystem::GetRangeRequest(handle, http_url, headers, file_offset, buffer_out, buffer_out_len);
782721
}
783722

784723
unique_ptr<HTTPResponse> S3FileSystem::DeleteRequest(FileHandle &handle, string s3_url, HTTPHeaders header_map) {
785724
auto auth_params = handle.Cast<S3FileHandle>().auth_params;
786725
auto parsed_s3_url = S3UrlParse(s3_url, auth_params);
787726
string http_url = parsed_s3_url.GetHTTPUrl(auth_params);
788-
789-
HTTPHeaders headers;
790-
if (IsGCSRequest(s3_url) && !auth_params.oauth2_bearer_token.empty()) {
791-
// Use bearer token for GCS
792-
headers["Authorization"] = "Bearer " + auth_params.oauth2_bearer_token;
793-
headers["Host"] = parsed_s3_url.host;
794-
} else {
795-
// Use existing S3 authentication
796-
headers = create_s3_header(parsed_s3_url.path, "", parsed_s3_url.host,
797-
"s3", "DELETE", auth_params, "", "", "", "");
798-
}
799-
727+
auto headers =
728+
create_s3_header(parsed_s3_url.path, "", parsed_s3_url.host, "s3", "DELETE", auth_params, "", "", "", "");
800729
return HTTPFileSystem::DeleteRequest(handle, http_url, headers);
801730
}
802731

@@ -845,12 +774,7 @@ void S3FileHandle::Initialize(optional_ptr<FileOpener> opener) {
845774
}
846775
if (entry->second == "403") {
847776
// 403: FORBIDDEN
848-
string extra_text;
849-
if (IsGCSRequest(path)) {
850-
extra_text = S3FileSystem::GetGCSAuthError(auth_params);
851-
} else {
852-
extra_text = S3FileSystem::GetS3AuthError(auth_params);
853-
}
777+
auto extra_text = S3FileSystem::GetS3AuthError(auth_params);
854778
throw Exception(error.Type(), error.RawMessage() + extra_text, extra_info);
855779
}
856780
}
@@ -1112,24 +1036,6 @@ string S3FileSystem::GetS3AuthError(S3AuthParams &s3_auth_params) {
11121036
return extra_text;
11131037
}
11141038

1115-
string S3FileSystem::GetGCSAuthError(S3AuthParams &s3_auth_params) {
1116-
string extra_text = "\n\nAuthentication Failure - GCS authentication failed.";
1117-
if (s3_auth_params.oauth2_bearer_token.empty() &&
1118-
s3_auth_params.secret_access_key.empty() &&
1119-
s3_auth_params.access_key_id.empty()) {
1120-
extra_text += "\n* No credentials provided.";
1121-
extra_text += "\n* For OAuth2: CREATE SECRET (TYPE GCS, bearer_token 'your-token')";
1122-
extra_text += "\n* For HMAC: CREATE SECRET (TYPE GCS, key_id 'key', secret 'secret')";
1123-
} else if (!s3_auth_params.oauth2_bearer_token.empty()) {
1124-
extra_text += "\n* Bearer token was provided but authentication failed.";
1125-
extra_text += "\n* Ensure your OAuth2 token is valid and not expired.";
1126-
} else {
1127-
extra_text += "\n* HMAC credentials were provided but authentication failed.";
1128-
extra_text += "\n* Ensure your HMAC key_id and secret are correct.";
1129-
}
1130-
return extra_text;
1131-
}
1132-
11331039
HTTPException S3FileSystem::GetS3Error(S3AuthParams &s3_auth_params, const HTTPResponse &response, const string &url) {
11341040
string extra_text;
11351041
if (response.status == HTTPStatusCode::BadRequest_400) {
@@ -1145,15 +1051,6 @@ HTTPException S3FileSystem::GetS3Error(S3AuthParams &s3_auth_params, const HTTPR
11451051

11461052
HTTPException S3FileSystem::GetHTTPError(FileHandle &handle, const HTTPResponse &response, const string &url) {
11471053
auto &s3_handle = handle.Cast<S3FileHandle>();
1148-
1149-
// Use GCS-specific error for GCS URLs
1150-
if (IsGCSRequest(url) && response.status == HTTPStatusCode::Forbidden_403) {
1151-
string extra_text = GetGCSAuthError(s3_handle.auth_params);
1152-
auto status_message = HTTPFSUtil::GetStatusMessage(response.status);
1153-
throw HTTPException(response, "HTTP error on '%s' (HTTP %d %s)%s", url,
1154-
response.status, status_message, extra_text);
1155-
}
1156-
11571054
return GetS3Error(s3_handle.auth_params, response, url);
11581055
}
11591056
string AWSListObjectV2::Request(string &path, HTTPParams &http_params, S3AuthParams &s3_auth_params,

0 commit comments

Comments
 (0)