armake2 icon indicating copy to clipboard operation
armake2 copied to clipboard

Handle UTF-8 errors when reading PBOs

Open BrettMayson opened this issue 6 years ago • 17 comments

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: FromUtf8Error { bytes: [82, 80, 95, 67, 111, 109, 109, 97, 110, 100, 115, 32, 151, 32, 107, 111, 112, 105, 97, 46, 104, 112, 112], error: Utf8Error { valid_up_to: 12, error_len: Some(1) } }', src/libcore/result.rs:997:5

Comes from read_cstring , one file I know that causes it is @rhsusaf/addons/rhsusf_c_radio.pbo while trying to use PBO::read or armake2 unpack

BrettMayson avatar Apr 27 '19 08:04 BrettMayson

It's being caused by an em dash (—) in the filename RP_Commands — kopia.hpp

BrettMayson avatar Apr 27 '19 09:04 BrettMayson

I determined this was being caused by the PBO being encoded with Windows-1252 filenames. The em dash (—) character is not able to be opened in a UTF-8 context and causes this bug.

BrettMayson avatar May 03 '19 02:05 BrettMayson

So is this even a real bug in armake then? :thinking:

Krzmbrzl avatar May 03 '19 05:05 Krzmbrzl

Yes, it can be handled

BrettMayson avatar May 03 '19 05:05 BrettMayson

But only if the encoding has some kind of identifying header, right? Is this the case?

Krzmbrzl avatar May 03 '19 10:05 Krzmbrzl

It can also be dropped by doing a lossy conversion to UTF-8, that is what I did for a workaround

BrettMayson avatar May 03 '19 10:05 BrettMayson

By that I assume you mean decoding the input with UTF-8 and throwing away all invalid bits and pieces that come up during that conversion?

Krzmbrzl avatar May 03 '19 12:05 Krzmbrzl

Yes

BrettMayson avatar May 03 '19 12:05 BrettMayson

Not sure if that is a good practice that should be applied in the general case. If this messes something up, then the user might get really confused because the error message complains about character sequences that don't exist in the original. Or even worse: armake doesn't even detect the error but some of the scripts messed up. This error would then be detected in Arma leading to some really awkward bugs.

I think it would be better to error out on such things. At least by default. I think we could add another option that can be set to force the lossy conersion in such cases.

Krzmbrzl avatar May 03 '19 15:05 Krzmbrzl

Handling this sloppily could lead to problems. Different tools handling this issue differently could lead to those tools calculating different hashes when signing or verifying PBOs, so a warning should be shown at least. Obviously it should be possible to handle it in some way since it would very simple to prevent unpacking of PBOs otherwise.

KoffeinFlummi avatar May 13 '19 18:05 KoffeinFlummi

We could add multiple tries to read the PBO in different encodings (Try UTF-8, error -> Try another).

As an alternative we could do a lossy conversion if the normal reading fails and somehow mark the file, like adding a fat comment on top

Soldia1138 avatar May 28 '19 20:05 Soldia1138

I think the problem is to detect whether decoding with the current charset has failed. It's not like it woul throw exceptions when attempting to use the wrong charset. You'd have to actually go through the decoded content and try to check whether this looks like proper text/code or whether that is rubbish...

Krzmbrzl avatar May 29 '19 04:05 Krzmbrzl

Using non lossy utf-8 will fail, at least with the RHS file from the original comment

BrettMayson avatar May 29 '19 05:05 BrettMayson

Maybe this here could be handy for this: https://github.com/lifthrasiir/rust-encoding We could iterate all charets that are available in that library and see if the decoding works properly. If not (from the README I assume that this library does also throw errors if the decoding fails), try the next one (and so on).

Krzmbrzl avatar May 29 '19 07:05 Krzmbrzl

That would be exactly what I meant. Good catch.

As I‘m doing a mass sign of files, I can say that probably 3-5% of all files (roughly 70GB of pbos) have non-UTF8 encoding. So it would be very beneficial, if armake2 would automatically switch through encodings if one fails.

I‘m wondering how armake1 handles this. Used that until now and it never throws any errors on this.

Soldia1138 avatar May 29 '19 07:05 Soldia1138

Another error but with packing
error: Failed to write PBO: Windows stdio in console mode does not support writing non-UTF-8 byte sequences

Reidond avatar Jul 21 '19 13:07 Reidond

The error seems also to occur when reading an LZSS compressed PBO

ArwynFr avatar May 03 '21 16:05 ArwynFr