lxsang / PTerm

MIT License
34 stars 8 forks source link

Terminal hangs on output that is not a valid UTF-8 encoding #28

Closed Rinzwind closed 2 years ago

Rinzwind commented 2 years ago

In the following example, the Terminal window ‘hangs’ on the second command, which produces output that is not a valid UTF-8 encoding:

PTerm Example Invalid UTF-8

This is due to the following:

https://github.com/lxsang/PTerm/blob/43fd24b90a5f95f7589681e2dc96569549791aa9/PTerm-Core/PTerm.class.st#L197-L202

For comparison, the example in xterm:

XTerm Example Invalid UTF-8
lxsang commented 2 years ago

can you attach your test.txt so that i can reproduce the problem.

Thanks

Rinzwind commented 2 years ago

Ah sorry, I forgot to include ‘test.txt’, here it is:

test.txt

Command that makes the same file:

printf '\106\162\141\156\347\141\151\163\012' >/tmp/test.txt

Command that outputs the same text, but in UTF-8:

printf '\106\162\141\156\303\247\141\151\163\012'

I think one thing to watch out for here is output like in the following in which the single UTF-8 sequence \303\247 is split:

printf '\106\162\141\156\303'; sleep 5; printf '\247\141\151\163\012'

Demo in xterm:

https://user-images.githubusercontent.com/1611248/173796691-13050995-c69c-4d6f-9c84-5d9ee5119d8e.mp4

lxsang commented 2 years ago

We can prevent the terminal hanging when there is an utf8 decoding error by resetting the output stream

stream nextPutAll: data.
    [
        data := stream contents decodeWith: #utf8.
        self announcer announce: (PTermDataEvent data: data ).
        stream reset.
    ] on:Error do:[:e|
        "self announcer announce: (PTermDataEvent data: e asString )."
        stream reset].

However i have no solution to display the ouput like traditional xTerm as ZnUTF8Encoder does not tolerate invalid utf8 bytes.

Maybe we should use another decoder or PTerm should have its own decoder

lxsang commented 2 years ago

I came up with a solution in the PR #32.

@Rinzwind could you review the changes?

Thanks

lxsang commented 2 years ago

PR #32 is merged