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

Sign encoded URLs #380

Merged

Conversation

tjcelaya
Copy link
Contributor

@tjcelaya tjcelaya commented Nov 23, 2017

Resolves #379 and https://trac.cyberduck.io/ticket/10151

We should be using the encoded URL for signing, e.g.:

/java.manta.ci/stor/%E2%9B%B0%20quack%20%F0%9F%A6%86

instead of

/java.manta.ci/stor/⛰ quack 🦆

Validate.isTrue(StringUtils.isEmpty(uri.getQuery()),
"Query must be null or empty. URI: %s", uri);
}
Validate.isTrue(StringUtils.isEmpty(uri.getQuery()),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

StringUtils.isEmpty returns true for null and empty strings

@@ -53,7 +56,7 @@ public void beforeClass() throws IOException {

mantaClient = new MantaClient(config);
testPathPrefix = IntegrationTestConfigContext.generateBasePath(config, this.getClass().getSimpleName());
mantaClient.putDirectory(testPathPrefix, null);
mantaClient.putDirectory(testPathPrefix, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test case can't be run on its own without changing this to recursive putDirectory

Copy link
Contributor

@dekobon dekobon left a comment

Choose a reason for hiding this comment

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

LGTM

@tjcelaya tjcelaya merged commit 8db8169 into TritonDataCenter:master Nov 27, 2017
@tjcelaya tjcelaya deleted the bug/379-signed-url-encoding branch November 27, 2017 19:16
@jussisallinen
Copy link

@tjcelaya Thanks a lot for the fix!

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.

None yet

3 participants