edmund-wagner / junrar

plain java unrar util (former sf project)
Other
186 stars 90 forks source link

Resolved decompressing issues with some rar files #36

Open RDamman opened 9 years ago

RDamman commented 9 years ago

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 commented 8 years ago

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

RDamman commented 8 years ago

You are right. It's not necessary.

phax commented 8 years ago

Do you know an active fork of this project?

RDamman commented 8 years ago

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

RDamman commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

https://github.com/RDamman/junrar/releases/tag/0.8.0