Skip to content

Commit 9e6804b

Browse files
committed
fix(libstore): preserve S3 query parameters for region support
S3 buckets in non-default regions require specifying the region to avoid 301 PermanentRedirect errors. This commit fixes two issues: 1. Query parameters in S3 store URLs (like ?region=us-east-2) were being treated as store settings instead of URL parameters, causing "unknown setting" warnings. 2. URL construction in makeRequest() was incorrectly concatenating paths with query parameters. The fix: - HttpBinaryCacheStoreConfig now preserves query parameters for S3 URLs - makeRequest() properly constructs URLs preserving query parameters without corrupting the path Now users can specify: s3://bucket?region=us-east-2 to access buckets in specific AWS regions.
1 parent f167bda commit 9e6804b

File tree

1 file changed

+14
-4
lines changed

1 file changed

+14
-4
lines changed

src/libstore/http-binary-cache-store.cc

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig(
3333
{
3434
while (!cacheUri.path.empty() && cacheUri.path.back() == '/')
3535
cacheUri.path.pop_back();
36+
37+
// For S3 stores, preserve query parameters as part of the URL
38+
// These are needed for region specification and other S3-specific settings
39+
if (scheme == "s3" && !params.empty()) {
40+
cacheUri.query = params;
41+
}
3642
}
3743

3844
StoreReference HttpBinaryCacheStoreConfig::getReference() const
@@ -170,10 +176,14 @@ class HttpBinaryCacheStore : public virtual BinaryCacheStore
170176
`std::filesystem::path`'s equivalent operator, which properly
171177
combines the the URLs, whether the right is relative or
172178
absolute. */
173-
return FileTransferRequest(
174-
hasPrefix(path, "https://") || hasPrefix(path, "http://") || hasPrefix(path, "file://")
175-
? path
176-
: config->cacheUri.to_string() + "/" + path);
179+
if (hasPrefix(path, "https://") || hasPrefix(path, "http://") || hasPrefix(path, "file://")) {
180+
return FileTransferRequest(path);
181+
} else {
182+
// Properly construct URL preserving query parameters
183+
auto baseUrl = config->cacheUri;
184+
baseUrl.path = baseUrl.path + "/" + path;
185+
return FileTransferRequest(baseUrl.to_string());
186+
}
177187
}
178188

179189
void getFile(const std::string & path, Sink & sink) override

0 commit comments

Comments
 (0)