Skip to content
This repository has been archived by the owner on Jul 21, 2020. It is now read-only.

Resolved decompressing issues with some rar files #36

Open
RDamman opened this issue Jul 29, 2015 · 7 comments
Open

Resolved decompressing issues with some rar files #36

RDamman opened this issue Jul 29, 2015 · 7 comments

Comments

@RDamman
Copy link

RDamman commented Jul 29, 2015

I downloaded the code of JUnrar 4 years ago because the author of HelloNzb refused to activate JUnrar in his program. This was because JUnrar seemed to have problems with certain rar files. So I downloaded the C code of unrar and tried to debug the Java code of JUnrar. In the process I updated the code of JUnrar a bit to be more on par with the downloaded C version of the code. So there have been a few updates in that area (see changes in unpack.java). I did not work on it for 4 years, but today I thought it was time to finally settle the issues with JUnrar. So what steps did I take:
1: I replaced all >> operators by >>> operators. This because the Java equivalent of the C >> operator is the >>> operator. Using the >> operator in Java, causes the sign bit to be left inserted when shifting right. This did not solve the problems in my testcase (but I did leave the modifications in place), so we did go further with the next step:
2: Extensively debugging. After hours of hard work, this brought the solution. There was a bug in RarVM.java. Look at line 932:

if (curByte==0xe8 || curByte==cmpByte2)

especially "curByte==0xe8". curByte is of type Byte and the value range is -128 to 127 in Java. So the condition will always be false. This is not a problem when filterType is VMSF_E8. But in the case of filterType is VMSF_E8E9, the javacode behaves different then the C counterpart. So I changed it to:

if (curByte==((byte)0xe8) || curByte==cmpByte2)

I searched the code on simular bugs and found another one in MarkHeader.java line 70:

else if (d[1]==0x61 && d[2]==0x72 && d[3]==0x21 && d[4]==0x1a &&

is changed in

else if (d[1]==0x61 && d[2]==0x72 && d[3]==0x21 && d[4]==((byte)0x1a) &&

The first modification in step 2 already fixed my problems with JUnrar. So I hope you will accept my changes for the repository.

With kind regards,

Roy Damman

albertus82 added a commit to albertus82/junrar that referenced this issue Jan 28, 2016
edmund-wagner/junrar#36

I downloaded the code of JUnrar 4 years ago because the author of
HelloNzb refused to activate JUnrar in his program. This was because
JUnrar seemed to have problems with certain rar files. So I downloaded
the C code of unrar and tried to debug the Java code of JUnrar. In the
process I updated the code of JUnrar a bit to be more on par with the
downloaded C version of the code. So there have been a few updates in
that area (see changes in unpack.java). I did not work on it for 4
years, but today I thought it was time to finally settle the issues with
JUnrar. So what steps did I take:
1: I replaced all >> operators by >>> operators. This because the Java
equivalent of the C >> operator is the >>> operator. Using the >>
operator in Java, causes the sign bit to be left inserted when shifting
right. This did not solve the problems in my testcase (but I did leave
the modifications in place), so we did go further with the next step:
2: Extensively debugging. After hours of hard work, this brought the
solution. There was a bug in RarVM.java. Look at line 932:

if (curByte==0xe8 || curByte==cmpByte2)

especially "curByte==0xe8". curByte is of type Byte and the value range
is -128 to 127 in Java. So the condition will always be false. This is
not a problem when filterType is VMSF_E8. But in the case of filterType
is VMSF_E8E9, the javacode behaves different then the C counterpart. So
I changed it to:

if (curByte==((byte)0xe8) || curByte==cmpByte2)

I searched the code on simular bugs and found another one in
MarkHeader.java line 70:

else if (d[1]==0x61 && d[2]==0x72 && d[3]==0x21 && d[4]==0x1a &&

is changed in

else if (d[1]==0x61 && d[2]==0x72 && d[3]==0x21 && d[4]==((byte)0x1a) &&

The first modification in step 2 already fixed my problems with JUnrar.
So I hope you will accept my changes for the repository.

With kind regards,

Roy Damman
@phax
Copy link

phax commented Apr 20, 2016

Good point :) But why did you add the cast to "0x1a"??? It's < 127

phax added a commit to phax/junrar that referenced this issue Apr 20, 2016
@RDamman
Copy link
Author

RDamman commented Apr 21, 2016

You are right. It's not necessary.

@phax
Copy link

phax commented Apr 21, 2016

Do you know an active fork of this project?

@RDamman
Copy link
Author

RDamman commented Apr 21, 2016

Maybe a review of all search results of the regular expression "0x[89ABCDEF][0-9ABCDEF][^0-9ABCDEF]" will be wise.

@RDamman
Copy link
Author

RDamman commented Apr 21, 2016

No, I was planning to create my own fork, but still haven't uploaded my changes. There are also a few performance issues in the code that can be improved.

@phax
Copy link

phax commented Apr 21, 2016

I'm currently not using the library so I don't want to spend too much time on it, but an active fork would be really appreciated :)

@RDamman
Copy link
Author

RDamman commented Apr 23, 2016 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants