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

[SSHD-1303] AbstractClientChannel: use null stream for redirected stderr #255

Merged
merged 1 commit into from Oct 21, 2022

Conversation

tomaswolf
Copy link
Member

If a channel's error stream has been redirected to the output stream, return null streams from getAsyncErr() and getInvertedErr(). Since they are redirected, application code should never see any data on them.

Also document the IoReadFuture better, and change the single implementation of that interface to match. While the future is not done, operations return null or zero. EOF is signaled by getRead() == -1, or getException() returning an EOFException.

If a channel's error stream has been redirected to the output stream,
return null streams from getAsyncErr() and getInvertedErr(). Since they
are redirected, application code should never see any data on them.

Also document the IoReadFuture better, and change the single
implementation of that interface to match. While the future is not
done, operations return null or zero. EOF is signaled by
getRead() == -1, or getException() returning an EOFException.
@@ -474,4 +494,48 @@ public Integer getExitStatus() {
public String getExitSignal() {
return exitSignalHolder.get();
}

private enum NullIoInputStream implements IoInputStream {
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend making this a public class in same package as IoInputStream or a member of it. Might come useful in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then let's delay doing so to that future until a second concrete use case for this comes up. YAGNI. Which basically means "implement features when they're actually needed, not when you think they might be needed."

@lgoldstein
Copy link
Contributor

Then let's delay doing so to that future until a second concrete use case for this comes up

My personal philosophy is that if something is relatively simple, generic enough to be useful then why not make it public.

YAGNI

I agree in principle, though it has been my experience that usually you are going to need it.

That being said, I have no issue with leaving this code as-is - like I said, I recommend it, but do not insist on it...

@tomaswolf tomaswolf merged commit 77b97c8 into apache:master Oct 21, 2022
@tomaswolf tomaswolf deleted the sshd-1303 branch November 2, 2022 22:39
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

2 participants