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

Support for out or err stream for the terminal (fixes #787) #788

Merged
merged 4 commits into from
Jan 16, 2023

Conversation

gnodet
Copy link
Member

@gnodet gnodet commented Jun 9, 2022

No description provided.

@gnodet gnodet changed the title Support for out or err stream for the terminal Support for out or err stream for the terminal (fixes #787) Jun 9, 2022
@gnodet gnodet marked this pull request as draft June 9, 2022 15:17
ProcessBuilder.Redirect input = stream == Stream.Input
? ProcessBuilder.Redirect.INHERIT
: getRedirect(stream == Stream.Output ? FileDescriptor.out : FileDescriptor.err);
Process p = new ProcessBuilder(OSUtils.TTY_COMMAND).redirectInput(input).start();
Copy link

Choose a reason for hiding this comment

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

COMMAND_INJECTION: This usage of java/lang/ProcessBuilder.([Ljava/lang/String;)V can be vulnerable to Command Injection

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -73,36 +89,39 @@ public static JansiWinSysTerminal createTerminal(String name, String type, boole
if (Kernel32.GetConsoleMode(consoleIn, mode) == 0) {
throw new IOException("Failed to get console mode: " + getLastErrorMessage());
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.terminal.impl.jansi.win.JansiWinConsoleWriter acquired by call to JansiWinConsoleWriter() at line 67 is not released after line 90.

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

try {
NonBlockingInputStream is = NonBlocking.nonBlocking( name, pty.getSlaveInput());
NonBlockingReader reader = NonBlocking.nonBlocking( name, is, StandardCharsets.UTF_8);
ForkJoinTask<Integer> t = new ForkJoinPool(1).submit(() -> is.read(1) );
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.utils.NonBlockingInputStreamImpl acquired by call to nonBlocking(...) at line 166 is not released after line 168.

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -73,36 +89,39 @@ public static JansiWinSysTerminal createTerminal(String name, String type, boole
if (Kernel32.GetConsoleMode(consoleIn, mode) == 0) {
throw new IOException("Failed to get console mode: " + getLastErrorMessage());
}
JansiWinSysTerminal terminal = new JansiWinSysTerminal(writer, name, type, encoding, codepage, nativeSignals, signalHandler);
JansiWinSysTerminal terminal = new JansiWinSysTerminal(writer, name, type, encoding, codepage, nativeSignals,
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.terminal.impl.jansi.win.JansiWinConsoleWriter acquired by call to JansiWinConsoleWriter() at line 81 is not released after line 92.

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Attributes attr = enterRawMode(pty);
try {
NonBlockingInputStream is = NonBlocking.nonBlocking( name, pty.getSlaveInput());
NonBlockingReader reader = NonBlocking.nonBlocking( name, is, StandardCharsets.UTF_8);
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.utils.NonBlocking$NonBlockingInputStreamReader acquired by call to nonBlocking(...) at line 167 is not released after line 167.

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -74,21 +74,21 @@ public static boolean isAtLeast(int major, int minor) {
}

@Override
public Pty current() throws IOException {
public Pty current(Stream consoleStream) throws IOException {
String osName = System.getProperty("os.name");
if (osName.startsWith("Linux")) {
Copy link

Choose a reason for hiding this comment

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

NULL_DEREFERENCE: object osName last assigned on line 78 could be null and is dereferenced at line 79.

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

ProcessBuilder.Redirect input = stream == Stream.Input
? ProcessBuilder.Redirect.INHERIT
: getRedirect(stream == Stream.Output ? FileDescriptor.out : FileDescriptor.err);
Process p = new ProcessBuilder(OSUtils.TTY_COMMAND).redirectInput(input).start();
Copy link

Choose a reason for hiding this comment

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

COMMAND_INJECTION: This usage of java/lang/ProcessBuilder.([Ljava/lang/String;)V can be vulnerable to Command Injection

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

return "jansi";
}

public Pty current( Stream consoleStream) throws IOException {
String osName = System.getProperty("os.name");
if (osName.startsWith("Linux")) {
Copy link

Choose a reason for hiding this comment

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

NULL_DEREFERENCE: object osName last assigned on line 85 could be null and is dereferenced at line 86.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -73,36 +89,39 @@ public static JansiWinSysTerminal createTerminal(String name, String type, boole
if (Kernel32.GetConsoleMode(consoleIn, mode) == 0) {
throw new IOException("Failed to get console mode: " + getLastErrorMessage());
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.terminal.impl.jansi.win.JansiWinConsoleWriter acquired by call to JansiWinConsoleWriter() at line 67 is not released after line 90.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -73,36 +89,39 @@ public static JansiWinSysTerminal createTerminal(String name, String type, boole
if (Kernel32.GetConsoleMode(consoleIn, mode) == 0) {
throw new IOException("Failed to get console mode: " + getLastErrorMessage());
}
JansiWinSysTerminal terminal = new JansiWinSysTerminal(writer, name, type, encoding, codepage, nativeSignals, signalHandler);
JansiWinSysTerminal terminal = new JansiWinSysTerminal(writer, name, type, encoding, codepage, nativeSignals,
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.terminal.impl.jansi.win.JansiWinConsoleWriter acquired by call to JansiWinConsoleWriter() at line 81 is not released after line 92.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@gnodet gnodet marked this pull request as ready for review June 13, 2022 12:23
@@ -109,7 +109,7 @@ private void applyAttribute() throws IOException {
if (underline) {
attributes |= BACKGROUND_INTENSITY;
Copy link

Choose a reason for hiding this comment

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

NarrowingCompoundAssignment: Compound assignments from int to short hide lossy casts

Suggested change
attributes |= BACKGROUND_INTENSITY;
attributes = (short) (attributes | BACKGROUND_INTENSITY);

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

}
}
if (Kernel32.GetConsoleMode(consoleIn, mode) == 0) {
throw new IOException("Failed to get console mode: " + getLastErrorMessage());
}
JansiWinSysTerminal terminal = new JansiWinSysTerminal(writer, name, type, encoding, codepage, nativeSignals, signalHandler);
JansiWinSysTerminal terminal = new JansiWinSysTerminal(writer, name, type, encoding, nativeSignals,
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.terminal.impl.jansi.win.JansiWinConsoleWriter acquired by call to JansiWinConsoleWriter(...) at line 69 is not released after line 94.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sb.append("The terminal seems to work: ");
sb.append("terminal ").append(terminal.getClass().getName());
if (terminal instanceof AbstractPosixTerminal) {
sb.append(" with pty ").append(((AbstractPosixTerminal) terminal).getPty().getClass().getName());
Copy link

Choose a reason for hiding this comment

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

NULL_DEREFERENCE: object returned by terminal.getPty() could be null and is dereferenced at line 112.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

} else {
if (type == null) {
type = TYPE_WINDOWS;
}
writer = new WindowsAnsiWriter(new BufferedWriter(new JansiWinConsoleWriter()));
writer = new WindowsAnsiWriter(new BufferedWriter(new JansiWinConsoleWriter(console)));
}
}
if (Kernel32.GetConsoleMode(consoleIn, mode) == 0) {
throw new IOException("Failed to get console mode: " + getLastErrorMessage());
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.terminal.impl.jansi.win.JansiWinConsoleWriter acquired by call to JansiWinConsoleWriter(...) at line 69 is not released after line 92.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

ProcessBuilder.Redirect input = stream == Stream.Input
? ProcessBuilder.Redirect.INHERIT
: getRedirect(stream == Stream.Output ? FileDescriptor.out : FileDescriptor.err);
Process p = new ProcessBuilder(OSUtils.TTY_COMMAND).redirectInput(input).start();
Copy link

Choose a reason for hiding this comment

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

COMMAND_INJECTION: This usage of java/lang/ProcessBuilder.([Ljava/lang/String;)V can be vulnerable to Command Injection

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]


public boolean isPosixSystemStream(Stream stream) {
try {
Process p = new ProcessBuilder(OSUtils.TEST_COMMAND, "-t", Integer.toString(stream.ordinal()))
Copy link

Choose a reason for hiding this comment

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

COMMAND_INJECTION: This usage of java/lang/ProcessBuilder.([Ljava/lang/String;)V can be vulnerable to Command Injection

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

} else {
if (type == null) {
type = TYPE_WINDOWS;
}
writer = new WindowsAnsiWriter(new BufferedWriter(new JansiWinConsoleWriter()));
writer = new WindowsAnsiWriter(new BufferedWriter(newConsoleWriter(console)));
}
}
if (Kernel32.GetConsoleMode(consoleIn, mode) == 0) {
throw new IOException("Failed to get console mode: " + getLastErrorMessage());
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.terminal.impl.jansi.win.JansiWinConsoleWriter acquired by call to newConsoleWriter(...) at line 68 is not released after line 91.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -116,14 +122,23 @@ else if (osName.startsWith("FreeBSD")) {
Copy link

Choose a reason for hiding this comment

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

NULL_DEREFERENCE: object osName last assigned on line 106 could be null and is dereferenced at line 107.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

}
}
if (Kernel32.GetConsoleMode(consoleIn, mode) == 0) {
throw new IOException("Failed to get console mode: " + getLastErrorMessage());
}
JansiWinSysTerminal terminal = new JansiWinSysTerminal(writer, name, type, encoding, codepage, nativeSignals, signalHandler);
JansiWinSysTerminal terminal = new JansiWinSysTerminal(writer, name, type, encoding, nativeSignals,
Copy link

Choose a reason for hiding this comment

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

RESOURCE_LEAK: resource of type org.jline.terminal.impl.jansi.win.JansiWinConsoleWriter acquired by call to newConsoleWriter(...) at line 82 is not released after line 93.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

return "jansi";
}

public Pty current(Stream consoleStream) throws IOException {
String osName = System.getProperty("os.name");
if (osName.startsWith("Linux")) {
Copy link

Choose a reason for hiding this comment

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

NULL_DEREFERENCE: object osName last assigned on line 85 could be null and is dereferenced at line 86.

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Copy link
Member Author

Choose a reason for hiding this comment

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

@sonatype-lift ignoreall

Copy link

Choose a reason for hiding this comment

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

The ignoreall command is active on this PR, all the existing Lift issues are ignored.

@gnodet gnodet added this to the 3.22.0 milestone Jan 16, 2023
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.

1 participant