Skip to content

fix: broken FilterInputStream in StreamConnectionProvider#forwardCopyTo#1352

Merged
mickaelistria merged 1 commit intoeclipse-lsp4e:mainfrom
sebthom:StreamConnectionProvider
Oct 30, 2025
Merged

fix: broken FilterInputStream in StreamConnectionProvider#forwardCopyTo#1352
mickaelistria merged 1 commit intoeclipse-lsp4e:mainfrom
sebthom:StreamConnectionProvider

Conversation

@sebthom
Copy link
Copy Markdown
Member

@sebthom sebthom commented Oct 23, 2025

StreamConnectionProvider#forwardCopyTo instantiates an anonymous FilterInputStream implementation with broken read method implementations (EOF handling). The FilterInputStream also unnecessarily allocates new byte arrays on each array read operation. Additionally the single byte read method did not forward to the output stream but wrote the byte directly to stderr.

@sebthom sebthom force-pushed the StreamConnectionProvider branch from 44ecffa to e48fb3c Compare October 23, 2025 10:03
@sebthom sebthom force-pushed the StreamConnectionProvider branch from e48fb3c to aa6db97 Compare October 23, 2025 10:05
@sebthom
Copy link
Copy Markdown
Member Author

sebthom commented Oct 23, 2025

@mickaelistria can you please have a look. you contributed the file initially and I would like your opinion on the hard coded System.err.print((char) res); in the original code as well as the reason why System.arraycopy in this synchronous code.

@sebthom sebthom requested a review from mickaelistria October 30, 2025 07:44
@mickaelistria
Copy link
Copy Markdown
Contributor

That's quite old code. I think it just happened to work in most cases and was never re-worked.
The change you're suggesting seems fine, and better. Let's merge then.

@mickaelistria mickaelistria merged commit 47718a7 into eclipse-lsp4e:main Oct 30, 2025
11 checks passed
@sebthom sebthom deleted the StreamConnectionProvider branch November 9, 2025 18:48
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.

2 participants