You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by "thachlp (via GitHub)" <gi...@apache.org> on 2023/03/25 08:31:01 UTC

[GitHub] [mina-sshd] thachlp opened a new pull request, #347: Update docs

thachlp opened a new pull request, #347:
URL: https://github.com/apache/mina-sshd/pull/347

   Motivation:
   There is no constructor like: `ProcessShellFactory(String[] command)`
   The docs should be update


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org


[GitHub] [mina-sshd] thachlp commented on a diff in pull request #347: Update docs

Posted by "thachlp (via GitHub)" <gi...@apache.org>.
thachlp commented on code in PR #347:
URL: https://github.com/apache/mina-sshd/pull/347#discussion_r1214096086


##########
docs/server-setup.md:
##########
@@ -45,8 +45,7 @@ implementation that you can use if you want. This implementation will create a p
 so it's mostly useful to launch the OS native shell. E.g.,
 
 ```java
-sshd.setShellFactory(new ProcessShellFactory(new String[] { "/bin/sh", "-i", "-l" }));
-
+sshServer.setShellFactory(new ProcessShellFactory("/bin/sh -i -l", "-i", "-l" ));
 ```

Review Comment:
   So we should fix the implementation. After trying what you suggested, it didn't work for me. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org


[GitHub] [mina-sshd] tomaswolf commented on a diff in pull request #347: Update docs

Posted by "tomaswolf (via GitHub)" <gi...@apache.org>.
tomaswolf commented on code in PR #347:
URL: https://github.com/apache/mina-sshd/pull/347#discussion_r1214522184


##########
docs/server-setup.md:
##########
@@ -45,8 +45,7 @@ implementation that you can use if you want. This implementation will create a p
 so it's mostly useful to launch the OS native shell. E.g.,
 
 ```java
-sshd.setShellFactory(new ProcessShellFactory(new String[] { "/bin/sh", "-i", "-l" }));
-
+sshServer.setShellFactory(new ProcessShellFactory("/bin/sh -i -l", "-i", "-l" ));
 ```

Review Comment:
   It's worse, but understandable if you look at existing uses. `command` is supposed to be the full command as it was received, `elements` is supposed to be the individual parts. The correct invocation is
   ```
   new ProcessShellFactory("/bin/sh -i -l", "/bin/sh", "-i", "-l")
   ```
   (This is from memory; I'm not at my development machine where I could easily double-check this.)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org


[GitHub] [mina-sshd] thachlp commented on a diff in pull request #347: Update docs

Posted by "thachlp (via GitHub)" <gi...@apache.org>.
thachlp commented on code in PR #347:
URL: https://github.com/apache/mina-sshd/pull/347#discussion_r1148394766


##########
docs/server-setup.md:
##########
@@ -45,8 +45,7 @@ implementation that you can use if you want. This implementation will create a p
 so it's mostly useful to launch the OS native shell. E.g.,
 
 ```java
-sshd.setShellFactory(new ProcessShellFactory(new String[] { "/bin/sh", "-i", "-l" }));
-
+sshServer.setShellFactory(new ProcessShellFactory("/bin/sh -i -l", "-i", "-l" ));
 ```

Review Comment:
   I don't fully understand but this configuration works, so the docs should be updated. The implementation also needs to be improved this too.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org


[GitHub] [mina-sshd] thachlp closed pull request #347: Update docs

Posted by "thachlp (via GitHub)" <gi...@apache.org>.
thachlp closed pull request #347: Update docs
URL: https://github.com/apache/mina-sshd/pull/347


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org


[GitHub] [mina-sshd] thachlp commented on a diff in pull request #347: Update docs

Posted by "thachlp (via GitHub)" <gi...@apache.org>.
thachlp commented on code in PR #347:
URL: https://github.com/apache/mina-sshd/pull/347#discussion_r1148394766


##########
docs/server-setup.md:
##########
@@ -45,8 +45,7 @@ implementation that you can use if you want. This implementation will create a p
 so it's mostly useful to launch the OS native shell. E.g.,
 
 ```java
-sshd.setShellFactory(new ProcessShellFactory(new String[] { "/bin/sh", "-i", "-l" }));
-
+sshServer.setShellFactory(new ProcessShellFactory("/bin/sh -i -l", "-i", "-l" ));
 ```

Review Comment:
   Don't fully understand but this configuration works. I also need to improve this too.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org


[GitHub] [mina-sshd] lgoldstein commented on a diff in pull request #347: Update docs

Posted by "lgoldstein (via GitHub)" <gi...@apache.org>.
lgoldstein commented on code in PR #347:
URL: https://github.com/apache/mina-sshd/pull/347#discussion_r1213600038


##########
docs/server-setup.md:
##########
@@ -45,8 +45,7 @@ implementation that you can use if you want. This implementation will create a p
 so it's mostly useful to launch the OS native shell. E.g.,
 
 ```java
-sshd.setShellFactory(new ProcessShellFactory(new String[] { "/bin/sh", "-i", "-l" }));
-
+sshServer.setShellFactory(new ProcessShellFactory("/bin/sh -i -l", "-i", "-l" ));
 ```

Review Comment:
   The fix seems wrong - the constructor is defined as
   ```java
   ProcessShellFactory(String command, String... elements) { ... }
   ```
   which means that one could use either
   ```java
   new ProcessShellFactory("/bin/sh", new String[] {"-i", "-l"})
   ```
   or
   ```java
   new ProcessShellFactory("/bin/sh", "-i", "-l" ))
   ```
   but *not*
   ```java
   new ProcessShellFactory("/bin/sh -i -l", "-i", "-l" )
   ```
   since it repeats the command arguments



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org