Fix escape-keys by preserving input if invalid #22943
Conversation
vdemeester
changed the title
vdemeester
force-pushed
the
21769-fix-detach-keys
branch
from
f8c7933
to
6a5513e
Compare
|
+1 see: #15666 (comment) |
|
Heh, it works. Bit of a weird side-effect is that typing |
|
any chance of getting some testcases? |
vdemeester
force-pushed
the
21769-fix-detach-keys
branch
from
6a5513e
to
44e74b7
Compare
|
@duglin I'm thinking of it, but I'm not sure how still... 😓 |
|
Perhaps you can add the "test" case to the commit message at least |
|
@vdemeester you may get some inspiration from the tests that use a tty - e.g. TestRunAttachDetach |
|
@duglin oh you are right. Remembered the way we pty to send input and detach the container but I did not remembered if we check the output content or not… and it seems we do |
Currently, using a custom detach key with an invalid sequence, eats a part of the sequence, making it weird and difficult to enter some key sequence. This fixes by keeping the input read when trying to see if it's the key sequence or not, and "writing" then is the key sequence is not the right one, preserving the initial input. Signed-off-by: Vincent Demeester <vincent@sbr.pm>
vdemeester
force-pushed
the
21769-fix-detach-keys
branch
from
44e74b7
to
0fb6190
Compare
|
Updated with tests 😉 |
|
LGTM |
|
LGTM |
| @@ -484,7 +484,9 @@ func copyEscapable(dst io.Writer, src io.ReadCloser, keys []byte) (written int64 | |||
| nr, er := src.Read(buf) | |||
| if nr > 0 { | |||
| // ---- Docker addition | |||
| preservBuf := []byte{} | |||
There was a problem hiding this comment.
Trying to follow the flow here, but it seems to me that "preservBuf" shouldn't be needed since it MUST match the keys, right? meaning if we're on the 3rd char of 'keys' then preservBuf must be the same as the first 2 chars of keys, right?
There was a problem hiding this comment.
@duglin hum it matches a part of the key, true, until the last wrong key is entered. Could have done something with keys, i and the latest read but it failed a little less readable to me 😅
There was a problem hiding this comment.
yea, I just wanted to avoid all of the alloc's and appends as we work our way thru the input.
Currently, using a custom detach key with an invalid sequence, eats a part of the sequence, making it weird and difficult to enter some key sequence. This fixes by keeping the input read when trying to see if it's the key sequence or not, and "writing" then is the key sequence is not the right one, preserving the initial input. (backport from moby#22943) Signed-off-by: Vincent Demeester <vincent@sbr.pm> Signed-off-by: Wentao Zhang <zhangwentao234@huawei.com> (cherry-pick from 0fb6190)
Fix escape-keys by preserving input if invalid Currently, using a custom detach key with an invalid sequence, eats a part of the sequence, making it weird and difficult to enter some key sequence. This fixes by keeping the input read when trying to see if it's the key sequence or not, and "writing" then is the key sequence is not the right one, preserving the initial input. <br> (backport from <a href="moby#22943" rel="nofollow noreferrer" target="_blank">https://github.com/moby/moby/pull/22943</a>) <br> Signed-off-by: Vincent Demeester <a href="mailto:vincent@sbr.pm">vincent@sbr.pm</a> Signed-off-by: Wentao Zhang <a href="mailto:zhangwentao234@huawei.com">zhangwentao234@huawei.com</a> (cherry-pick from <a href="/docker/docker/commit/0fb6190243d6101f96283e487cd4911142a05483" data-original="0fb6190243d6101f96283e487cd4911142a05483" data-link="false" data-project="7713" data-commit="0fb6190243d6101f96283e487cd4911142a05483" data-reference-type="commit" title="" class="gfm gfm-commit has-tooltip" data-original-title="Fix escape-keys by preserving input if invalid">0fb61902</a>) See merge request docker/docker!629
Currently, using a custom detach key with an invalid sequence, eats a part of the sequence, making it weird and difficult to enter some key sequence 🐯.
This fixes by keeping the input read when trying to see if it's the key sequence or not, and "writing" then is the key sequence is not the right one, preserving the initial input 🐹.
Fixes #21769
/cc @thaJeztah @cpuguy83 @LK4D4 @runcom @calavera
🐸
Signed-off-by: Vincent Demeester vincent@sbr.pm