Teditor icon indicating copy to clipboard operation
Teditor copied to clipboard

ted crashs/hangs on binary files [bug]

Open bynect opened this issue 4 years ago • 31 comments

I erroneously opened a binary file in ted and it behaved strangely (misplaced cursor and line numbers), and after a certain line (300 or something) just hanged. I have yet to reproduce this behavior on multiple files, but I will post updates on this bug soon.

bynect avatar Mar 30 '21 16:03 bynect

Maybe its a problem with the utf8 encoding/decoding mechanism

bynect avatar Mar 30 '21 16:03 bynect

Maybe its a problem with the utf8 encoding/decoding mechanism

Try opening without utf8

arthurbacci avatar Mar 30 '21 16:03 arthurbacci

Sorry, closed for accident.

arthurbacci avatar Mar 30 '21 16:03 arthurbacci

Maybe its a problem with the utf8 encoding/decoding mechanism

Try opening without utf8

Is there a way to disable utf8?

bynect avatar Mar 30 '21 16:03 bynect

askjaf That's were the UTF8 is read

arthurbacci avatar Mar 30 '21 16:03 arthurbacci

justcomment Just comment this part of the code

arthurbacci avatar Mar 30 '21 16:03 arthurbacci

Ok I'll try this asap

bynect avatar Mar 30 '21 16:03 bynect

Ok I tried it and seems like that when commenting that part of the code, lines are no longer misplaced, but now ted hangs on line ~450. ~~But maybe this problem is more related to memory (?)~~ Now I tried opening the ted binary itself and it runs completely fine (I scrolled down to line 1500), so maybe its not a memory problem after all.

bynect avatar Mar 30 '21 16:03 bynect

Maybe the problem lies within a call to utf8ToMultibyte from show_lines, but I am unsure on how to handle that

bynect avatar Mar 30 '21 17:03 bynect

I have retried and ted reads successfully till the end of the binary file, however it still misplaces the cursor sometimes and up to a certain point becomes so clunky I can't even tell if its crashed or just slow, so this seems like a bug to me

bynect avatar Mar 30 '21 17:03 bynect

I think we need a system to ignore characters that don't represent anything in text.

arthurbacci avatar Mar 30 '21 19:03 arthurbacci

I think we need a system to ignore characters that don't represent anything in text.

By replacing with something like this (https://www.compart.com/en/unicode/U+16844) ? I think its a very good idea, and even production grade editors do that.

bynect avatar Mar 30 '21 20:03 bynect

The worst problem is that in case of a malformed unicode sequence we could read arrays out of bound or end up in other UB situations

bynect avatar Mar 30 '21 20:03 bynect

Also I have successfully read over 5000 lines with ted, so this theory about memory is officially disproved.

Ok I tried it and seems like that when commenting that part of the code, lines are no longer misplaced, but now ted hangs on line ~450. ~But maybe this problem is more related to memory (?)~ Now I tried opening the ted binary itself and it runs completely fine (I scrolled down to line 1500), so maybe its not a memory problem after all.

bynect avatar Mar 30 '21 20:03 bynect

Also I have successfully read over 5000 lines with ted, so this theory about memory is officially disproved.

Ok I tried it and seems like that when commenting that part of the code, lines are no longer misplaced, but now ted hangs on line ~450. ~But maybe this problem is more related to memory (?)~ Now I tried opening the ted binary itself and it runs completely fine (I scrolled down to line 1500), so maybe its not a memory problem after all.

In theory, ted can read ~4gb

arthurbacci avatar Mar 30 '21 20:03 arthurbacci

The worst problem is that in case of a malformed unicode sequence we could read arrays out of bound or end up in other UB situations

I'll look into this, quoting from rfc 3629

Implementations of the decoding algorithm above MUST protect against decoding invalid sequences. For instance, a naive implementation may decode the overlong UTF-8 sequence C0 80 into the character U+0000, or the surrogate pair ED A1 8C ED BE B4 into U+233B4. Decoding invalid sequences may have security consequences or cause other problems. See Security Considerations (Section 10) below.

bynect avatar Mar 30 '21 21:03 bynect

errorted This is an example of the line numbers/cursor misplacement

bynect avatar Mar 30 '21 22:03 bynect

I have tried to add the utf8 validation but I am unsure if it is actually working well, I would appreciate if you can check here https://github.com/bynect/Teditor/blob/utf8try/src/utf8.c (I have made only small changes)

I tried to replace all invalid codepoints with this one

bynect avatar Mar 30 '21 22:03 bynect

errorted This is an example of the line numbers/cursor misplacement

I think it can be a problem with the line break type detection. Maybe it detects it as CRLF, but a loose LF is not interpreted as a line?

arthurbacci avatar Mar 30 '21 22:03 arthurbacci

errorted This is an example of the line numbers/cursor misplacement

I think it can be a problem with the line break type detection. Maybe it detects it as CRLF, but a loose LF is not interpreted as a line?

I cut it out from the screenshot, but the file was detected as lf, and to my knowledge all files, binary or not, are lf terminated on Linux

bynect avatar Mar 30 '21 22:03 bynect

errorted This is an example of the line numbers/cursor misplacement

I think it can be a problem with the line break type detection. Maybe it detects it as CRLF, but a loose LF is not interpreted as a line?

I cut it out from the screenshot, but the file was detected as lf, and to my knowledge all files, binary or not, are lf terminated on Linux

If you download a file that was made in Windows, it probally will be CRLF.

arthurbacci avatar Mar 30 '21 22:03 arthurbacci

errorted This is an example of the line numbers/cursor misplacement

I think it can be a problem with the line break type detection. Maybe it detects it as CRLF, but a loose LF is not interpreted as a line?

I cut it out from the screenshot, but the file was detected as lf, and to my knowledge all files, binary or not, are lf terminated on Linux

If you download a file that was made in Windows, it probally will be CRLF.

Well yes, but in this case the file was produced on Linux, specifically by gcc from a little test c code

bynect avatar Mar 30 '21 22:03 bynect

I will take a look at it later, now I am changing other things in the code.

arthurbacci avatar Mar 30 '21 22:03 arthurbacci

Regarding this I think I have just found the root cause of misplaced lines et al, which is the way read_lines read linebreaks. Ive also found out that binary files are CRLF terminated even on linux, and this problem is related to that

bynect avatar Mar 31 '21 12:03 bynect

Probably the problem is that ted is trying to find the type of the line break, while it does not have a consistent line break type.

arthurbacci avatar Mar 31 '21 13:03 arthurbacci

Probably the problem is that ted is trying to find the type of the line break, while it does not have a consistent line break type.

Exactly, furthermore I found that read_lines discards a character after having found a carriage return without checking for the linefeed in CRLF mode. Also I was trying to implement an heuristics that would guess in an acceptable manner if the file is valid utf8/non binary.

bynect avatar Mar 31 '21 13:03 bynect

If we remove CR line break support, we can just ignore carriage returns when reading the file.

arthurbacci avatar Mar 31 '21 14:03 arthurbacci

CR is not used in any modern operating system. There is no why to support it.

arthurbacci avatar Mar 31 '21 14:03 arthurbacci

If we remove CR line break support, we can just ignore carriage returns when reading the file.

https://github.com/ArthurBacci64/Teditor/commit/48523c4f7bbbe27536dd41644aed2edf345f1537

arthurbacci avatar Aug 04 '21 22:08 arthurbacci