Skip to content

Commit dfd228e

Browse files
otter606otter606
committed
Fixed length argument bug, addressed PR comments
1 parent 718abdb commit dfd228e

3 files changed

Lines changed: 45 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ BioJava 6.0.0 (future release)
3131
* Moved all chem-comp model classes from `org.biojava.nbio.structure.io.mmcif.chem` to `org.biojava.nbio.structure.chem`
3232
* Moved all chem-comp parsing classes from `org.biojava.nbio.structure.io.mmcif.chem` to `org.biojava.nbio.structure.io.cif`
3333
* Moved classes in `org.biojava.nbio.structure.io.mmcif` to `org.biojava.nbio.structure.chem`
34+
* Fixed `CRC64Checksum#public void update(byte[] b, int offset, int length)` to use
35+
the `length` argument correctly as specified in `java.util.zip.Checksum` interface.
3436

3537
### Fixed
3638
* Correct chain assignment to entities when parsing PDB/mmCIF without entity information (in cases with more than 3 chains per entity) #931

‎biojava-core/src/main/java/org/biojava/nbio/core/util/CRC64Checksum.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,27 @@ public void update(int b) {
5656
crc = low ^ high;
5757
}
5858

59-
// condition loop should be i < offset + length
59+
/**
60+
* Updates the CRC-64 checksum with the specified array of bytes.
61+
* <br/>
62+
* Note that BioJava before version 6.0 implemented this method incorrectly,
63+
* using {@code length} as an index.
64+
*
65+
* @throws IllegalArgumentException
66+
* if {@code offset} is negative, or {@code length} is negative, or
67+
* {@code offset+length} is negative or greater than the length of
68+
* the array {@code b}.
69+
*/
6070
@Override
6171
public void update(byte[] b, int offset, int length) {
62-
for (int i = offset; i < length; ++i)
72+
if (b == null) {
73+
throw new IllegalArgumentException("byte array cannot be null");
74+
}
75+
if (offset < 0 || length < 0 || offset > b.length - length) {
76+
throw new IllegalArgumentException("Offset and length must be non-negative"+
77+
" and their sum cannot be greater than length of byte array");
78+
}
79+
for (int i = offset; i < length + offset; ++i)
6380
update(b[i]);
6481
}
6582

‎biojava-core/src/test/java/org/biojava/nbio/core/util/CRC64ChecksumTest.java‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package org.biojava.nbio.core.util;
22

3+
import static org.junit.jupiter.api.Assertions.assertAll;
34
import static org.junit.jupiter.api.Assertions.assertEquals;
5+
import static org.junit.jupiter.api.Assertions.assertThrows;
46

57
import org.junit.jupiter.api.BeforeEach;
6-
import org.junit.jupiter.api.Disabled;
78
import org.junit.jupiter.api.DisplayName;
89
import org.junit.jupiter.api.RepeatedTest;
910
import org.junit.jupiter.api.Test;
@@ -75,20 +76,36 @@ void allRangeIsSameAsAllArray (){
7576
}
7677

7778
@Test
78-
@Disabled
79-
// this doesn't work as expected
80-
// and doesn't behave according to interface. The 3rd argument
81-
// is treated as an index rather than a number of bytes to include
82-
void partialbyteRange (){
79+
void partialByteRange (){
8380
byte [] testBytes = new byte [] {1,2,3,4,5};
84-
// should update with testBytes[2] but doesn't
8581
crc64.update(testBytes, 2, 1);
8682
String partialBytesHex = crc64.toString();
8783
crc64.reset();
8884
crc64.update(testBytes[2]);
8985
assertEquals(partialBytesHex, crc64.toString());
9086
}
9187

88+
@Test
89+
void partialByteRangeRejectsInvalidInput (){
90+
byte [] testBytes = new byte [] {1,2,3,4,5};
91+
assertAll(
92+
()->assertThrows(IllegalArgumentException.class,
93+
()->crc64.update(testBytes, -1, 0)),
94+
()->assertThrows(IllegalArgumentException.class,
95+
()->crc64.update(testBytes, 0, -1)),
96+
()->assertThrows(IllegalArgumentException.class,
97+
()->crc64.update(testBytes, 0, testBytes.length+1)),
98+
()->assertThrows(IllegalArgumentException.class,
99+
()->crc64.update(testBytes, testBytes.length, 1))
100+
);
101+
crc64.update(testBytes, 2, 1);
102+
String partialBytesHex = crc64.toString();
103+
crc64.reset();
104+
crc64.update(testBytes[2]);
105+
assertEquals(partialBytesHex, crc64.toString());
106+
}
107+
108+
92109
@Test
93110
void hexStringIsEqualToValue(){
94111
Long value = Long.parseLong(helloInCrc64Hex, 16);

0 commit comments

Comments
 (0)