Skip to content

Commit 4a20b44

Browse files
gab1onectrueden
authored andcommitted
ReadBufferDataHandle: ensure pages are fully read
Reading bytes with `DataHandle#read(bytes[], off, len)` results in *up to* `len` bytes being read into the provided array. In ReadBufferHandleDataHandle the number of read bytes was not checked, this lead to partially filled pages resulting in read errors. Now the reading is repeated until the page is either full or EOF is reached. And adapt to changes in ReadBufferDataHandle.
1 parent ae05d6b commit 4a20b44

File tree

2 files changed

+37
-15
lines changed

2 files changed

+37
-15
lines changed

src/main/java/org/scijava/io/handle/ReadBufferDataHandle.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,17 @@ private byte[] readPage(final int pageID, final int slotID) throws IOException {
167167
if (handle().offset() != startOfPage) {
168168
handle().seek(startOfPage);
169169
}
170-
handle().read(page);
170+
171+
// NB: we read repeatedly until the page is full or EOF is reached
172+
// handle().read(..) might read less bytes than requested
173+
int off = 0;
174+
while (off < pageSize) {
175+
final int read = handle().read(page, off, pageSize - off);
176+
if (read == -1) { // EOF
177+
break;
178+
}
179+
off += read;
180+
}
171181
return page;
172182
}
173183

src/test/java/org/scijava/io/handle/ReadBufferDataHandleMockTest.java

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,25 @@
11

22
package org.scijava.io.handle;
33

4-
import org.junit.Before;
5-
import org.junit.Test;
6-
74
import static org.junit.Assert.assertEquals;
85
import static org.mockito.AdditionalMatchers.aryEq;
96
import static org.mockito.ArgumentMatchers.any;
7+
import static org.mockito.ArgumentMatchers.anyInt;
108
import static org.mockito.ArgumentMatchers.anyLong;
9+
import static org.mockito.ArgumentMatchers.eq;
10+
import static org.mockito.Mockito.doAnswer;
11+
import static org.mockito.Mockito.mock;
12+
import static org.mockito.Mockito.times;
13+
import static org.mockito.Mockito.verify;
14+
import static org.mockito.Mockito.when;
1115

16+
import java.io.IOException;
17+
18+
import org.junit.Before;
19+
import org.junit.Test;
1220
import org.scijava.io.location.DummyLocation;
1321
import org.scijava.io.location.Location;
1422

15-
import static org.mockito.Mockito.*;
16-
17-
import java.io.IOException;
18-
1923
public class ReadBufferDataHandleMockTest {
2024

2125
private DataHandle<Location> mock;
@@ -59,34 +63,40 @@ public void testBufferingSequence() throws IOException {
5963

6064
// set length of stubbed handle
6165
when(mock.length()).thenReturn(30l);
66+
byte[] value = new byte[10];
67+
when(mock.read(aryEq(value), eq(0), eq(10))).thenReturn(10);
68+
when(mock.read(aryEq(value), anyInt(), anyInt())).thenReturn(10);
6269

6370
// read the first byte
6471
buf.read();
6572
verify(mock, times(0)).seek(0);
6673
// buffer should read a whole page
67-
verify(mock).read(aryEq(byteArrayLen10));
74+
verify(mock).read(aryEq(byteArrayLen10), eq(0), eq(10));
6875

6976
buf.seek(0);
7077
// ensure seek was not called again
7178
verify(mock, times(0)).seek(0);
7279

80+
when(mock.offset()).thenReturn(10l);
81+
7382
// read over the edge of the current page
7483
buf.read(new byte[12]);
7584
verify(mock, times(0)).seek(anyLong());
76-
verify(mock, times(2)).read(aryEq(byteArrayLen10));
85+
verify(mock, times(2)).read(aryEq(byteArrayLen10), eq(0), eq(10));
7786

7887
assertEquals(12, buf.offset());
7988

8089
// read the last page
90+
when(mock.offset()).thenReturn(20l);
8191
buf.read(new byte[12]);
8292
verify(mock, times(0)).seek(anyLong());
83-
verify(mock, times(3)).read(aryEq(byteArrayLen10));
93+
verify(mock, times(3)).read(aryEq(byteArrayLen10), eq(0), eq(10));
8494

8595
// first page should no longer be buffered, must be reread in
8696
buf.seek(0);
8797
buf.read();
8898
verify(mock).seek(0);
89-
verify(mock, times(4)).read(aryEq(byteArrayLen10));
99+
verify(mock, times(4)).read(aryEq(byteArrayLen10), eq(0), eq(10));
90100
}
91101

92102
/**
@@ -99,19 +109,21 @@ public void testSkipForward() throws IOException {
99109

100110
// set length of stubbed handle
101111
when(mock.length()).thenReturn(40l);
112+
when(mock.read(any(), anyInt(), anyInt())).thenReturn(10);
102113

103114
// read the first byte
104115
buf.read();
105116
verify(mock, times(0)).seek(anyLong());
106-
verify(mock).read(aryEq(byteArrayLen10));
117+
verify(mock, times(1)).read(aryEq(byteArrayLen10), eq(0), eq(10));
107118

108119
// skip the second page
109120
buf.seek(30l);
110121
buf.read();
111122

112123
// read the third page
113124
verify(mock).seek(30l);
114-
verify(mock, times(2)).read(aryEq(byteArrayLen10));
125+
verify(mock, times(2)).read(aryEq(byteArrayLen10), eq(0), eq(10));
126+
when(mock.offset()).thenReturn(40l);
115127

116128
// go back to already buffered page
117129
buf.seek(0l);
@@ -123,6 +135,6 @@ public void testSkipForward() throws IOException {
123135
buf.seek(35);
124136
buf.read();
125137
verify(mock, times(1)).seek(anyLong());
126-
verify(mock, times(2)).read(any());
138+
verify(mock, times(2)).read(any(), anyInt(), anyInt());
127139
}
128140
}

0 commit comments

Comments
 (0)