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

systemd: Use binary channel for Terminal #20840

Merged
merged 3 commits into from
Aug 6, 2024

Conversation

martinpitt
Copy link
Member

Terminals are not defined/required to be UTF-8 only. Sometimes people
cat binary data to the terminal, or possibly files in other encodings.
These cannot be sent through TEXT websocket channels, and will also
trigger the additional checks from commit 1b15dcb.

Use a binary channel to avoid all that. XTerm.js gets along with this
just fine, and it ignores invalid data.

With this you can now even cat /bin/true and get something moderately
sensible, instead of no output at all.

Fixes #20791

Commit 1b15dcb did not cover a "completely" binary channel with
no UTF-8 data at all. Also try to read that binary http endpoint as text
channel and ensure the channel gets closed properly with `protocol-error`.
Unlike most other code paths, raising a ChannelError in data_received()
did not get translated to a proper close message. In particular,  this
caused an unhandled bridge exception in __decode_frame() when a text
channel encountered non-UTF8 data since commit 1b15dcb.

Side issue in cockpit-project#20791
Terminals are not defined/required to be UTF-8 only. Sometimes people
cat binary data to the terminal, or possibly files in other encodings.
These cannot be sent through `TEXT` websocket channels, and will also
trigger the additional checks from commit 1b15dcb.

Use a binary channel to avoid all that. XTerm.js gets along with this
just fine, and it ignores invalid data.

With this you can now even `cat /bin/true` and get something moderately
sensible, instead of no output at all.

Fixes cockpit-project#20791
Copy link
Member

@mvollmer mvollmer left a comment

Choose a reason for hiding this comment

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

Thanks!

@mvollmer mvollmer merged commit eff7d97 into cockpit-project:main Aug 6, 2024
81 checks passed
@martinpitt martinpitt deleted the utf8 branch August 6, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Targetted for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UnicodeDecodeError: 'utf-8' codec can't decode byte 0xbd in position 6: invalid start byte
2 participants