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.

Sign up for GitHub

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

Fixes #164 #167

Merged
merged 6 commits into from
Sep 18, 2023
+42 −29
Merged

Fixes #164 #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
012a8a8
Fixes #164
dekobon Aug 17, 2023
ecfcb24
make the isDirectory check simpler and more inclusive of error states
4141done Aug 18, 2023
c1fe531
Allow useful output from curl and enable timeouts
dekobon Aug 23, 2023
13b79e9
Add nginx debug build container
dekobon Sep 6, 2023
eeda5ed
Fix latest-njs build in container
dekobon Sep 6, 2023
148609f
remove debug changes
4141done Sep 15, 2023
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
View file
Open in desktop
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;

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

/**
Expand Down
2 changes: 1 addition & 1 deletion common/etc/nginx/templates/default.conf.template
View file
Open in desktop
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;
}

# Provide a hint to the client on 405 errors of the acceptable request methods
Expand Down
5 changes: 5 additions & 0 deletions test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,11 @@ integration_test 2 0 1 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 "Test API with AWS Signature V4 and allow directory listing off"
integration_test 4 0 0 0

Expand Down
1 change: 1 addition & 0 deletions test/data/bucket-1/test/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>This is an index page of the d directory</h1>
26 changes: 19 additions & 7 deletions test/integration/test_api.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ if ! [ -x "${curl_cmd}" ]; then
e "required dependency not found: curl not found in the path or not executable"
exit ${no_dep_exit_code}
fi
curl_cmd="${curl_cmd} --connect-timeout 3 --max-time 30 --no-progress-meter"

# Allow for MacOS which does not support "md5sum"
# but has "md5 -r" which can be substituted
Expand Down Expand Up @@ -104,11 +105,11 @@ assertHttpRequestEquals() {

if [ "${method}" = "HEAD" ]; then
expected_response_code="$3"
actual_response_code="$(${curl_cmd} -s -o /dev/null -w '%{http_code}' --head "${uri}" ${extra_arg})"
actual_response_code="$(${curl_cmd} -o /dev/null -w '%{http_code}' --head "${uri}" ${extra_arg})"

if [ "${expected_response_code}" != "${actual_response_code}" ]; then
e "Response code didn't match expectation. Request [${method} ${uri}] Expected [${expected_response_code}] Actual [${actual_response_code}]"
e "curl command: ${curl_cmd} -s -o /dev/null -w '%{http_code}' --head '${uri}' ${extra_arg}"
e "curl command: ${curl_cmd} -o /dev/null -w '%{http_code}' --head '${uri}' ${extra_arg}"
exit ${test_fail_exit_code}
fi
elif [ "${method}" = "GET" ]; then
Expand All @@ -118,21 +119,21 @@ assertHttpRequestEquals() {
checksum_output="$(${checksum_cmd} "${body_data_path}")"
expected_checksum="${checksum_output:0:${checksum_length}}"

curl_checksum_output="$(${curl_cmd} -s -X "${method}" "${uri}" ${extra_arg} | ${checksum_cmd})"
curl_checksum_output="$(${curl_cmd} -X "${method}" "${uri}" ${extra_arg} | ${checksum_cmd})"
s3_file_checksum="${curl_checksum_output:0:${checksum_length}}"

if [ "${expected_checksum}" != "${s3_file_checksum}" ]; then
e "Checksum doesn't match expectation. Request [${method} ${uri}] Expected [${expected_checksum}] Actual [${s3_file_checksum}]"
e "curl command: ${curl_cmd} -s -X '${method}' '${uri}' ${extra_arg} | ${checksum_cmd}"
e "curl command: ${curl_cmd} -X '${method}' '${uri}' ${extra_arg} | ${checksum_cmd}"
exit ${test_fail_exit_code}
fi
else
expected_response_code="$3"
actual_response_code="$(${curl_cmd} -s -o /dev/null -w '%{http_code}' "${uri}" ${extra_arg})"
actual_response_code="$(${curl_cmd} -o /dev/null -w '%{http_code}' "${uri}" ${extra_arg})"

if [ "${expected_response_code}" != "${actual_response_code}" ]; then
e "Response code didn't match expectation. Request [${method} ${uri}] Expected [${expected_response_code}] Actual [${actual_response_code}]"
e "curl command: ${curl_cmd} -s -o /dev/null -w '%{http_code}' '${uri}' ${extra_arg}"
e "curl command: ${curl_cmd} -o /dev/null -w '%{http_code}' '${uri}' ${extra_arg}"
exit ${test_fail_exit_code}
fi
fi
Expand Down Expand Up @@ -288,6 +289,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 +309,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