Skip to content

Commit

Permalink
Fix edge case in GridFTP Authz where '/' is used
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshuaSBrown committed Jan 27, 2025
1 parent 45eb9bf commit 4f86a90
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 4 deletions.
2 changes: 1 addition & 1 deletion core/database/foxx/api/repo.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class Repo {
throw [g_lib.ERR_INTERNAL_FAULT, "Repo document is missing path: " + this.#repo_id];
}

// Get and sanitize the repo root path
// Get and sanitize the repo root path by removing the trailing slash if one exists
let repo_root_path = repo.path.replace(/\/$/, "");
let sanitized_path = a_path.replace(/\/$/, "");

Expand Down
12 changes: 12 additions & 0 deletions core/database/foxx/tests/repo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,16 @@ describe("Testing Repo class", () => {
const repo = new Repo("foo");
expect(repo.pathType("/mnt/repo_root/user/jane/4243")).to.equal(PathType.USER_RECORD_PATH);
});

it("unit_repo: should handle a user record path.", () => {
const path = "/mnt/datafed/compose-home/";
g_db.repo.save({
_id: "repo/foo",
_key: "foo",
path: path,
});
const repo = new Repo("foo");
expect(repo.pathType("/mnt/datafed/compose-home/user/tim/1135")).to.equal(PathType.USER_RECORD_PATH);
});

});
13 changes: 10 additions & 3 deletions repository/gridftp/globus5/authz/source/AuthzWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,10 +413,17 @@ std::string AuthzWorker::getAuthzPath(char *full_ftp_path) {
std::string local_path = removeOrigin(full_ftp_path);

if (isPathValid(local_path) == false) {
EXCEPT(1, "Invalid POSIX path.");
EXCEPT(1, "Invalid POSIX path: " + std::string(full_ftp_path) + " local_path: " + local_path);
}

std::string prefix = local_path.substr(0, m_local_globus_path_root.length());
if ( prefix.length() == 1) {
if (prefix.compare("/") == 0 ) {
return local_path;
} else {
EXCEPT(1, "Invalid POSIX path: " + std::string(full_ftp_path) + " local_path: " + local_path + " prefix: " + prefix);
}
}
auto prefix = local_path.substr(0, m_local_globus_path_root.length());

return local_path.substr(prefix.length());
}

Expand Down
24 changes: 24 additions & 0 deletions repository/gridftp/globus5/authz/tests/unit/test_AuthzWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,18 @@ BOOST_AUTO_TEST_CASE(InvalidURLMissingSchemeTest) {
BOOST_CHECK(worker.isURLValid(invalid_url) == false);
}

BOOST_AUTO_TEST_CASE(GetAuthzPathGlobusBaseSetToRoot) {
// Test a valid full FTP path when the globus_collection_path is /
SDMS::LogContext log_context;
config.globus_collection_path[0] = '/';
config.globus_collection_path[1] = '\0';
SDMS::AuthzWorker worker(&config, log_context);
char deep_path[] = "ftp://hostname/globus/root/a/b/c/d/e.txt";
std::string expected_result = "/globus/root/a/b/c/d/e.txt";

BOOST_CHECK_EQUAL(worker.getAuthzPath(deep_path), expected_result);
}

BOOST_AUTO_TEST_CASE(InvalidURLTooFewSlashesTest) {
SDMS::LogContext log_context;
SDMS::AuthzWorker worker(&config, log_context);
Expand Down Expand Up @@ -211,6 +223,18 @@ BOOST_AUTO_TEST_CASE(URLWithoutHostnameTest) {
BOOST_CHECK(worker.isURLValid(no_hostname_url) == false);
}

BOOST_AUTO_TEST_CASE(RemoveOriginGlobusBaseSetToRoot) {
// Test a valid full FTP path when the globus_collection_path is /
SDMS::LogContext log_context;
config.globus_collection_path[0] = '/';
config.globus_collection_path[1] = '\0';
SDMS::AuthzWorker worker(&config, log_context);
char deep_path[] = "ftp://hostname/globus/root/a/b/c/d/e.txt";
std::string expected_result = "/globus/root/a/b/c/d/e.txt";

BOOST_CHECK_EQUAL(worker.removeOrigin(deep_path), expected_result);
}

BOOST_AUTO_TEST_CASE(RemoveOriginValidURLTest) {
// Test a valid FTP URL
SDMS::LogContext log_context;
Expand Down

0 comments on commit 4f86a90

Please sign in to comment.