Skip to content

Comments

Support reading multiple bytes in escapeProxy#4

Merged
cpuguy83 merged 1 commit intomoby:masterfrom
thaJeztah:term_proxy_pty
Apr 16, 2020
Merged

Support reading multiple bytes in escapeProxy#4
cpuguy83 merged 1 commit intomoby:masterfrom
thaJeztah:term_proxy_pty

Conversation

@thaJeztah
Copy link
Member

this applies the patch from moby/moby#39728;

curl -fsSL https://patch-diff.githubusercontent.com/raw/moby/moby/pull/39728.patch | git am -p3 -S

patch applied cleanly


Currently, the escapeProxy works under the assumption that the
underlying reader will always return 1 byte at a time. Even though this
is usually true, it is not always the case, for example when using a pty
and writing multiple bytes to the master before flushing it.

In such cases the proxy reader doesn't work properly. For example with
an escape sequence being ctrl-p,ctrl-q, when the underlying reader
returns ctrl-p,ctrl-q at once, the escape sequence isn't detected.

This updates the reader to support this use-case and adds unit tests.

Signed-off-by: Bilal Amarni [email protected]

Currently, the escapeProxy works under the assumption that the
underlying reader will always return 1 byte at a time. Even though this
is usually true, it is not always the case, for example when using a pty
and writing multiple bytes to the master before flushing it.

In such cases the proxy reader doesn't work properly. For example with
an escape sequence being `ctrl-p,ctrl-q`, when the underlying reader
returns `ctrl-p,ctrl-q` at once, the escape sequence isn't detected.

This updates the reader to support this use-case and adds unit tests.

Signed-off-by: Bilal Amarni <[email protected]>
@thaJeztah
Copy link
Member Author

ping @cpuguy83 @dims @bamarni ptal

Copy link
Contributor

@bamarni bamarni left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83 cpuguy83 merged commit 063f2cd into moby:master Apr 16, 2020
@thaJeztah thaJeztah deleted the term_proxy_pty branch April 16, 2020 14:00
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.

4 participants