You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Daniel Estévez <no...@github.com> on 2018/05/11 16:56:19 UTC
[jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Passwords can contain special characters as parentheses, password parameter should be wrapped in '' to avoid this syntax errors in SSH command
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/1205
-- Commit Summary --
* Fixes Run SSH script for passwords with special characters (as parentheses)
-- File Changes --
M compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeUsingSsh.java (3)
M compute/src/test/java/org/jclouds/compute/callables/RunScriptOnNodeUsingSshTest.java (2)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/1205.patch
https://github.com/jclouds/jclouds/pull/1205.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Andrew Phillips <no...@github.com>.
demobox commented on this pull request.
Curious whether we need to extend the test coverage a bit here?
> @@ -103,7 +103,7 @@ public void simpleRootTestWithSudoPassword() {
expect(sshClient.getUsername()).andReturn("tester");
expect(sshClient.getHostAddress()).andReturn("somewhere.example.com");
expect(
- sshClient.exec("sudo -S sh <<'RUN_SCRIPT_AS_ROOT_SSH'\n" + "testpassword!\n" + "echo $USER\n"
+ sshClient.exec("sudo -S sh <<'RUN_SCRIPT_AS_ROOT_SSH'\n" + "'testpassword!'\n" + "echo $USER\n"
Would it make sense to modify this test so that it actually reflects the problem we're trying to fix (e.g. space in the password)? Also, would it be worth adding a test for the additional code path (see comment above)?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#pullrequestreview-119628362
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Daniel Estévez <no...@github.com>.
danielestevez commented on this pull request.
> @@ -103,7 +103,7 @@ public void simpleRootTestWithSudoPassword() {
expect(sshClient.getUsername()).andReturn("tester");
expect(sshClient.getHostAddress()).andReturn("somewhere.example.com");
expect(
- sshClient.exec("sudo -S sh <<'RUN_SCRIPT_AS_ROOT_SSH'\n" + "testpassword!\n" + "echo $USER\n"
+ sshClient.exec("sudo -S sh <<'RUN_SCRIPT_AS_ROOT_SSH'\n" + "'testpassword!'\n" + "echo $USER\n"
@demobox
That line of code https://github.com/jclouds/jclouds/pull/1205#discussion-diff-187774732R118 is already covered in simpleRootTest and testUserAddAsRoot tests
This PR adds a conflictive password for all tests https://github.com/jclouds/jclouds/pull/1207
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#discussion_r188094382
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> @@ -103,7 +103,7 @@ public void simpleRootTestWithSudoPassword() {
expect(sshClient.getUsername()).andReturn("tester");
expect(sshClient.getHostAddress()).andReturn("somewhere.example.com");
expect(
- sshClient.exec("sudo -S sh <<'RUN_SCRIPT_AS_ROOT_SSH'\n" + "testpassword!\n" + "echo $USER\n"
+ sshClient.exec("sudo -S sh <<'RUN_SCRIPT_AS_ROOT_SSH'\n" + "'testpassword!'\n" + "echo $USER\n"
+1. Let's change the default value used in tests to a conflicting one. @danielestevez can you have a look at these comments and submit a PR?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#discussion_r187871056
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Daniel Estévez <no...@github.com>.
It's just a sometimes since these kind of characters can be included in password random generator
https://github.com/danielestevez/jclouds/blob/9fef6ed06b027c1525579c88b58aa9f5833687fd/core/src/main/java/org/jclouds/util/PasswordGenerator.java#L65
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#issuecomment-388467721
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Daniel Estévez <no...@github.com>.
danielestevez commented on this pull request.
> @@ -112,7 +112,8 @@ protected ExecResponse runCommand(String command) {
public String execAsRoot(String command) {
if (node.getCredentials().identity.equals("root")) {
} else if (node.getCredentials().shouldAuthenticateSudo()) {
- command = String.format("sudo -S sh <<'%s'\n%s\n%s%s\n", MARKER, node.getCredentials().getOptionalPassword().get(), command, MARKER);
+ command = String.format("sudo -S sh <<'%s'\n'%s'\n%s%s\n", MARKER, node.getCredentials().getOptionalPassword
+ ().get(), command, MARKER);
} else {
command = String.format("sudo sh <<'%s'\n%s%s\n", MARKER, command, MARKER);
It shouldn't need one since the problem only happens when we submit a conflictive password
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#discussion_r188088462
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Ignasi Barrera <no...@github.com>.
Closed #1205.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#event-1622778189
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Andrew Phillips <no...@github.com>.
demobox commented on this pull request.
> @@ -112,7 +112,8 @@ protected ExecResponse runCommand(String command) {
public String execAsRoot(String command) {
if (node.getCredentials().identity.equals("root")) {
} else if (node.getCredentials().shouldAuthenticateSudo()) {
- command = String.format("sudo -S sh <<'%s'\n%s\n%s%s\n", MARKER, node.getCredentials().getOptionalPassword().get(), command, MARKER);
+ command = String.format("sudo -S sh <<'%s'\n'%s'\n%s%s\n", MARKER, node.getCredentials().getOptionalPassword
+ ().get(), command, MARKER);
} else {
command = String.format("sudo sh <<'%s'\n%s%s\n", MARKER, command, MARKER);
Does this line need a similar fix?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#pullrequestreview-119628326
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Ignasi Barrera <no...@github.com>.
Cherry-picked to [2.1.x](http://git-wip-us.apache.org/repos/asf/jclouds/commit/889a7f1d).
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#issuecomment-389768687
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to [master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/82289948). @andreaturli I think the change is safe enough to cherry-pick it to 2.1.x, WDYT?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#issuecomment-388483896
Re: [jclouds/jclouds] Fixes Run SSH script for passwords with special
characters (#1205)
Posted by Andrea Turli <no...@github.com>.
Haven't tested myself but lgtm, thanks @danielestevez
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1205#issuecomment-388466244