Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with the function readBytesTerm in "ByteBufferKaitaiStream.java" #35

Closed
ranitg opened this issue Feb 21, 2021 · 1 comment
Closed
Assignees
Labels

Comments

@ranitg
Copy link

ranitg commented Feb 21, 2021

Hi,

The line:
int c = bb.get();
in the function readBytesTerm in "ByteBufferKaitaiStream.java"

This should read into an unsigned int variable.

In the kaitai web IDE the code works as:
terminator: 0xfe

but in this project:
terminator: -2 (the byte is read as -2 (int) instead of 254 (unsigned int))

This is an inconsistency.

Thanks!

@generalmimon generalmimon self-assigned this Mar 21, 2021
@generalmimon
Copy link
Member

Good catch! I came to the conclusion that the signature of method readBytesTerm:

abstract public byte[] readBytesTerm(int term, boolean includeTerm, boolean consumeTerm, boolean eosError);

should be changed to accept byte in place of the term parameter (not int). It just makes more sense and conveys the intent better - terminator isn't allowed to be anything else than a byte value. In addition, methods for applying the terminator and pad-right properties on already parsed byte arrays (which arise when you use size or size-eos: true) also use the byte type to accept byte values:

public static byte[] bytesStripRight(byte[] bytes, byte padByte) {

public static byte[] bytesTerminate(byte[] bytes, byte term, boolean includeTerm) {

On the other hand, the processXor method for a single value uses int for the byte key (which is especially strange because the processXor for a multi-byte key is using byte[]):

public static byte[] processXor(byte[] data, int key) {

public static byte[] processXor(byte[] data, byte[] key) {

So I think it's reasonable to normalize everything to use byte everywhere.

There is a question of breaking backward compatibility, since we'll need to adjust the compiler to inject the (byte) 254 type castings (and the generated Java parsers intended for older runtime versions (without the narrowing type conversion) will trigger a compile-time error), but I believe it's not much of an issue - anyone can just grab the latest compiler and update the parser in minutes.

generalmimon added a commit to kaitai-io/kaitai_struct_tests that referenced this issue Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants