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

Conversation

4141done
Copy link
Collaborator

@4141done 4141done commented Apr 19, 2024

What

Aims to fix #210

The gateway has a feature that allows serving of the base index page that lists files in the directory if no index.html exists in the requested folder. In order to do this it performs a test request against <path>/ index.html to check for an existing index page.

For path-style URIs where the bucket name must be provided, this resulted in the bucket name being prepended to the request twice.

For more details on the presentation and required solution, please see the issue.

Fix

I wanted to keep s3uri in charge of the behavior here, so chose to add an options parameter to it with the option preserveBasePath so that we can ask it to do its other work, but give back a path without anything prepended. In the future this object could be used to offer control over other operations

… bucket name prepend for path-style requests
Comment on lines -292 to +316
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}`);
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

@4141done 4141done changed the base branch from master to main June 25, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Path style breaks fetching INDEX_PAGE
1 participant