Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue where bucket name is appended twice on path-style index page requests #230

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
33 changes: 25 additions & 8 deletions common/etc/nginx/include/s3gateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,30 +299,42 @@ function s3BaseUri(r) {
* Returns the s3 path given the incoming request
*
* @param r {NginxHTTPRequest} HTTP request
* @param opts {Object} Additional options for assembling the s3 URI
* @param opts.preserveBasePath {boolean} If set, modifications to the base path such as the addition of the bucket name will not be performed.
* @returns {string} uri for s3 request
*/
function s3uri(r) {
let uriPath = r.variables.uri_path;
const basePath = s3BaseUri(r);
function s3uri(r, opts) {
if (!opts) {
opts = { preserveBasePath: false };
}

let basePath;
let path;
let uriPath = r.variables.uri_path;

if (opts.preserveBasePath) {
basePath = '';
} else {
basePath = s3BaseUri(r);
}

// Create query parameters only if directory listing is enabled.
if (ALLOW_LISTING && !utils.parseBoolean(r.variables.forIndexPage)) {
const queryParams = _s3DirQueryParams(uriPath, r.method);
if (queryParams.length > 0) {
path = basePath + '?' + queryParams;
path = `${basePath}?${queryParams}`;
} else {
path = _escapeURIPath(basePath + uriPath);
path = _escapeURIPath(`${basePath}${uriPath}`);
}
} else {
// This is a path that will resolve to an index page
if (PROVIDE_INDEX_PAGE && _isDirectory(uriPath) ) {
uriPath += INDEX_PAGE;
}
path = _escapeURIPath(basePath + uriPath);
path = _escapeURIPath(`${basePath}${uriPath}`);
}

utils.debug_log(r, 'S3 Request URI: ' + r.method + ' ' + path);
utils.debug_log(r, `S3 Request URI: ${r.method} ${path}`);
Comment on lines -313 to +337
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annotation
These changes are not part of the main change - just some visual cleanup

return path;
}

Expand Down Expand Up @@ -418,7 +430,12 @@ async function loadContent(r) {
r.internalRedirect("@s3Directory");
return;
}
const uri = s3uri(r);
// For the index page check, don't add the bucket name
// in the case of a path-style uri because the subrequest will
// add it after the redirect. Adding it here would result in the
// bucket name being added twice.
const uri = s3uri(r, { preserveBasePath: true });

let reply = await ngx.fetch(
`http://127.0.0.1:80${uri}`
);
Expand Down
Loading