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

[Je issue/164 #169

Closed
wants to merge 3 commits into from
Closed
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
37 changes: 16 additions & 21 deletions common/etc/nginx/include/s3gateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ function redirectToS3(r) {

if (isDirectoryListing && (r.method === 'GET' || r.method === 'HEAD')) {
r.internalRedirect("@s3PreListing");
} else if ( PROVIDE_INDEX_PAGE == true ) {
} else if (PROVIDE_INDEX_PAGE === true) {
r.internalRedirect("@s3");
} else if ( !ALLOW_LISTING && !PROVIDE_INDEX_PAGE && uriPath == "/" ) {
} else if (!ALLOW_LISTING && !PROVIDE_INDEX_PAGE && uriPath === "/") {
r.internalRedirect("@error404");
} else {
r.internalRedirect("@s3");
Expand All @@ -333,8 +333,12 @@ function redirectToS3(r) {

function trailslashControl(r) {
if (APPEND_SLASH) {
// For the purposes of understanding whether this is a directory,
// consider the uri without query params or anchors
const path = r.variables.uri_path.split(/[?#]/)[0];

const hasExtension = /\/[^.\/]+\.[^.]+$/;
if (!hasExtension.test(r.variables.uri_path) && !_isDirectory(r.variables.uri_path)){
if (!hasExtension.test(path) && !_isDirectory(path)){
return r.internalRedirect("@trailslash");
}
}
Expand All @@ -353,22 +357,20 @@ async function loadContent(r) {
r.internalRedirect("@s3Directory");
return;
}
const url = s3uri(r);
const uri = s3uri(r);
let reply = await ngx.fetch(
`http://127.0.0.1:80${url}`
`http://127.0.0.1:80${uri}`
);

if (reply.status == 200) {
// found index.html, so redirect to it
r.internalRedirect(r.variables.request_uri + INDEX_PAGE);
} else if (reply.status == 404) {
// else just list the contents of the directory
if (reply.status === 200) {
utils.debug_log(r, `Found index file, redirecting to: ${uri}`);
r.internalRedirect(uri);
} else if (reply.status === 404) {
// As there was no index file found, just list the contents of the directory
r.internalRedirect("@s3Directory");
} else {
r.internalRedirect("@error500");
}

return;
}

/**
Expand Down Expand Up @@ -449,16 +451,9 @@ function _escapeURIPath(uri) {
* @private
*/
function _isDirectory(path) {
if (path === undefined) {
return false;
}
const len = path.length;

if (len < 1) {
return false;
}
if (!path) return false;
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:
Not a big deal, but the previous implementation didn't account for null. Perhaps that was on purpose? Either way, if we assume null, undefined, or "" are all "not a directory" this function gets a lot simpler.


return path.charAt(len - 1) === '/';
return path.slice(-1) === '/';
}

/**
Expand Down
2 changes: 1 addition & 1 deletion common/etc/nginx/templates/default.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ server {

location @trailslash {
# 302 to request without slashes
rewrite ^ $scheme://$http_host$request_uri/ redirect;
rewrite ^ $scheme://$http_host$uri/$is_args$query_string redirect;
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:
Insert the trailing slash after the path and preserve the query string after it.

}

# Provide a hint to the client on 405 errors of the acceptable request methods
Expand Down
5 changes: 5 additions & 0 deletions test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,11 @@ integration_test 2 1 0 0

compose stop nginx-s3-gateway # Restart with new config

p "Testing API with AWS Signature V2 and allow directory listing on and append slash and allow index"
integration_test 2 1 1 1

compose stop nginx-s3-gateway # Restart with new config

p "Testing API with AWS Signature V2 and static site on"
integration_test 2 0 1 0

Expand Down
1 change: 1 addition & 0 deletions test/data/bucket-1/test/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>This is an index page of the d directory</h1>
13 changes: 12 additions & 1 deletion test/integration/test_api.sh
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,15 @@ assertHttpRequestEquals "GET" "/statichost/noindexdir/multipledir/" "data/bucket
assertHttpRequestEquals "GET" "/statichost" "data/bucket-1/statichost/index.html"
assertHttpRequestEquals "GET" "/statichost/noindexdir/multipledir" "data/bucket-1/statichost/noindexdir/multipledir/index.html"
fi

if [ "${allow_directory_list}" == "1" ]; then
if [ "$append_slash" == "1" ]; then
assertHttpRequestEquals "GET" "test" "200"
assertHttpRequestEquals "GET" "test/" "200"
assertHttpRequestEquals "GET" "test?foo=bar" "200"
assertHttpRequestEquals "GET" "test/?foo=bar" "200"
fi
fi
fi

if [ "${allow_directory_list}" == "1" ]; then
Expand All @@ -299,7 +308,9 @@ if [ "${allow_directory_list}" == "1" ]; then
assertHttpRequestEquals "GET" "%D1%81%D0%B8%D1%81%D1%82%D0%B5%D0%BC%D1%8B/" "200"
assertHttpRequestEquals "GET" "системы/" "200"
if [ "$append_slash" == "1" ]; then
assertHttpRequestEquals "GET" "b" "302"
if [ "${index_page}" == "0" ]; then
assertHttpRequestEquals "GET" "b" "302"
fi
else
assertHttpRequestEquals "GET" "b" "404"
fi
Expand Down