Skip to content

Commit 0f4927c

Browse files
overheadhunteroverheadhunter
committed
Merge branch 'hotfix/write-zeros-when-skipping'
fixes #129
2 parents d9c8bef + afa6a88 commit 0f4927c

File tree

4 files changed

+126
-31
lines changed

4 files changed

+126
-31
lines changed

‎src/main/java/org/cryptomator/cryptofs/fh/ByteSource.java renamed to ‎src/main/java/org/cryptomator/cryptofs/ch/ByteSource.java

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,22 @@
66
* Contributors:
77
* Sebastian Stenzel - initial API and implementation
88
*******************************************************************************/
9-
package org.cryptomator.cryptofs.fh;
9+
package org.cryptomator.cryptofs.ch;
1010

11-
import static java.lang.Math.min;
11+
import org.cryptomator.cryptolib.common.ByteBuffers;
1212

1313
import java.nio.ByteBuffer;
1414

15-
public interface ByteSource {
15+
import static java.lang.Math.min;
16+
17+
interface ByteSource {
1618

1719
static ByteSource from(ByteBuffer buffer) {
1820
return new ByteBufferByteSource(buffer);
1921
}
2022

21-
static UndefinedNoisePrefixedByteSourceWithoutBuffer undefinedNoise(long numBytes) {
22-
return buffer -> new UndefinedNoisePrefixedByteSource(numBytes, buffer);
23+
static ZeroPrefixedByteSourceWithoutBuffer repeatingZeroes(long amountOfZeroes) {
24+
return buffer -> new ZeroPrefixedByteSource(amountOfZeroes, buffer);
2325
}
2426

2527
boolean hasRemaining();
@@ -36,7 +38,7 @@ static UndefinedNoisePrefixedByteSourceWithoutBuffer undefinedNoise(long numByte
3638
*/
3739
void copyTo(ByteBuffer buffer);
3840

39-
interface UndefinedNoisePrefixedByteSourceWithoutBuffer {
41+
interface ZeroPrefixedByteSourceWithoutBuffer {
4042

4143
ByteSource followedBy(ByteBuffer delegate);
4244

@@ -63,59 +65,59 @@ public long remaining() {
6365
@Override
6466
public void copyTo(ByteBuffer target) {
6567
if (source.remaining() > target.remaining()) {
66-
int originalLimit = source.limit();
67-
source.limit(source.position() + target.remaining());
68-
target.put(source);
69-
source.limit(originalLimit);
68+
ByteBuffers.copy(source, target);
7069
} else {
7170
target.put(source);
7271
}
7372
}
7473

7574
}
7675

77-
class UndefinedNoisePrefixedByteSource implements ByteSource {
76+
class ZeroPrefixedByteSource implements ByteSource {
77+
78+
private static final ByteBuffer ZEROES = ByteBuffer.allocate(4069);
7879

79-
private long prefixLen;
80+
private long amountOfZeroes;
8081
private final ByteBuffer source;
8182

82-
private UndefinedNoisePrefixedByteSource(long prefixLen, ByteBuffer source) {
83-
this.prefixLen = prefixLen;
83+
private ZeroPrefixedByteSource(long amountOfZeroes, ByteBuffer source) {
84+
this.amountOfZeroes = amountOfZeroes;
8485
this.source = source;
8586
}
8687

8788
@Override
8889
public boolean hasRemaining() {
89-
return prefixLen > 0 || source.hasRemaining();
90+
return amountOfZeroes > 0 || source.hasRemaining();
9091
}
9192

9293
@Override
9394
public long remaining() {
94-
return prefixLen + source.remaining();
95+
return amountOfZeroes + source.remaining();
9596
}
9697

9798
@Override
9899
public void copyTo(ByteBuffer target) {
99-
if (prefixLen > 0) {
100-
skip(target);
100+
while (amountOfZeroes > 0 && target.hasRemaining()) {
101+
copyZeroesTo(target);
101102
}
102103
if (target.hasRemaining()) {
103104
copySourceTo(target);
104105
}
105106
}
106107

107-
private void skip(ByteBuffer target) {
108-
int n = (int) min(prefixLen, target.remaining()); // known to fit into int due to 2nd param
109-
target.position(target.position() + n);
110-
prefixLen -= n;
108+
private void copyZeroesTo(ByteBuffer target) {
109+
var zeroes = ZEROES.asReadOnlyBuffer(); // use a view to protect original pos/limit
110+
int amountOfZeroesToCopy = (int) min(amountOfZeroes, zeroes.remaining());
111+
zeroes.limit(amountOfZeroesToCopy);
112+
amountOfZeroes -= ByteBuffers.copy(zeroes, target);
111113
}
112114

113115
private void copySourceTo(ByteBuffer target) {
114-
int originalLimit = source.limit();
115-
int limit = min(source.limit(), source.position() + target.remaining());
116-
source.limit(limit);
117-
target.put(source);
118-
source.limit(originalLimit);
116+
if (source.remaining() > target.remaining()) {
117+
ByteBuffers.copy(source, target);
118+
} else {
119+
target.put(source);
120+
}
119121
}
120122

121123
}

‎src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import org.cryptomator.cryptofs.CryptoFileSystemStats;
55
import org.cryptomator.cryptofs.EffectiveOpenOptions;
66
import org.cryptomator.cryptofs.fh.BufferPool;
7-
import org.cryptomator.cryptofs.fh.ByteSource;
87
import org.cryptomator.cryptofs.fh.ChunkCache;
98
import org.cryptomator.cryptofs.fh.Chunk;
109
import org.cryptomator.cryptofs.fh.ExceptionsDuringWrite;
@@ -124,7 +123,7 @@ protected int writeLocked(ByteBuffer src, long position) throws IOException {
124123
if (position > oldFileSize) {
125124
// we need to fill the gap:
126125
long gapLen = position - oldFileSize;
127-
final ByteSource byteSource = ByteSource.undefinedNoise(gapLen).followedBy(src); // prepend zeros to the original src
126+
final ByteSource byteSource = ByteSource.repeatingZeroes(gapLen).followedBy(src); // prepend zeros to the original src
128127
written = writeLockedInternal(byteSource, oldFileSize) - gapLen; // fill the gap by beginning to write from old EOF
129128
} else {
130129
final ByteSource byteSource = ByteSource.from(src);

‎src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package org.cryptomator.cryptofs;
1010

1111
import com.google.common.jimfs.Jimfs;
12+
import org.cryptomator.cryptofs.util.ByteBuffers;
1213
import org.cryptomator.cryptolib.api.Masterkey;
1314
import org.cryptomator.cryptolib.api.MasterkeyLoader;
1415
import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException;
@@ -24,12 +25,14 @@
2425
import org.junit.jupiter.api.io.TempDir;
2526
import org.junit.jupiter.params.ParameterizedTest;
2627
import org.junit.jupiter.params.provider.CsvSource;
28+
import org.junit.jupiter.params.provider.MethodSource;
2729
import org.junit.jupiter.params.provider.ValueSource;
2830
import org.mockito.Mockito;
2931

3032
import java.io.IOException;
3133
import java.net.URI;
3234
import java.nio.ByteBuffer;
35+
import java.nio.channels.Channels;
3336
import java.nio.channels.FileChannel;
3437
import java.nio.channels.FileLock;
3538
import java.nio.charset.StandardCharsets;
@@ -39,6 +42,8 @@
3942
import java.nio.file.attribute.FileTime;
4043
import java.time.Instant;
4144
import java.time.temporal.ChronoUnit;
45+
import java.util.List;
46+
import java.util.stream.Stream;
4247

4348
import static java.lang.Math.min;
4449
import static java.lang.String.format;
@@ -49,7 +54,6 @@
4954
import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING;
5055
import static java.nio.file.StandardOpenOption.WRITE;
5156
import static org.cryptomator.cryptofs.CryptoFileSystemProperties.cryptoFileSystemProperties;
52-
import static org.cryptomator.cryptofs.CryptoFileSystemUri.create;
5357
import static org.cryptomator.cryptofs.util.ByteBuffers.repeat;
5458

5559
public class CryptoFileChannelWriteReadIntegrationTest {
@@ -260,6 +264,54 @@ public void testFileSizeIsTenAfterWritingTenBytes() throws IOException {
260264
Assertions.assertEquals(10, Files.size(file));
261265
}
262266

267+
268+
// tests https://github.com/cryptomator/cryptofs/issues/129
269+
@ParameterizedTest
270+
@MethodSource
271+
public void testSkipBeyondEofAndWrite(List<SparseContent> contentRanges) throws IOException {
272+
// write sparse file
273+
try (var ch = FileChannel.open(file, CREATE, WRITE)) {
274+
for (var range : contentRanges) {
275+
var buf = ByteBuffers.repeat(range.pattern).times(range.len).asByteBuffer();
276+
ch.write(buf, range.pos);
277+
}
278+
}
279+
280+
// read and compare to expected
281+
try (var ch = FileChannel.open(file, READ); var in = Channels.newInputStream(ch)) {
282+
int pos = 0;
283+
for (var range : contentRanges) {
284+
// verify gaps are zeroes:
285+
for (int p = pos; p < range.pos; p++) {
286+
if (in.read() != 0) {
287+
Assertions.fail("Expected NIL byte at pos " + p);
288+
}
289+
}
290+
// verify expected values
291+
for (int p = 0; p < range.len; p++) {
292+
if (in.read() != range.pattern) {
293+
Assertions.fail("Expected byte at pos " + (pos + p) + " to be " + range.pattern);
294+
}
295+
}
296+
pos = range.pos + range.len;
297+
}
298+
}
299+
}
300+
301+
private record SparseContent(byte pattern, int pos, int len) {
302+
303+
}
304+
305+
public Stream<List<SparseContent>> testSkipBeyondEofAndWrite() {
306+
return Stream.of( //
307+
List.of(new SparseContent((byte) 0x01, 50, 100)), //
308+
List.of(new SparseContent((byte) 0x01, 0, 1000), new SparseContent((byte) 0x02, 20_000, 1000)), //
309+
List.of(new SparseContent((byte) 0x01, 0, 1000), new SparseContent((byte) 0x02, 36_000, 1000)), //
310+
List.of(new SparseContent((byte) 0x01, 2_000_000, 84_000), new SparseContent((byte) 0x02, 3_000_000, 10_000)), //
311+
List.of(new SparseContent((byte) 0x01, 50, 100), new SparseContent((byte) 0x02, 250, 100), new SparseContent((byte) 0x03, 450, 100), new SparseContent((byte) 0x04, 20_000, 1000), new SparseContent((byte) 0x05, 3_000_000, 10_000)) //
312+
);
313+
}
314+
263315
@Test
264316
public void testWriteAndReadNothing() throws IOException {
265317
try (FileChannel channel = FileChannel.open(file, CREATE, WRITE)) {

‎src/test/java/org/cryptomator/cryptofs/fh/ByteSourceTest.java renamed to ‎src/test/java/org/cryptomator/cryptofs/ch/ByteSourceTest.java

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66
* Contributors:
77
* Sebastian Stenzel - initial API and implementation
88
*******************************************************************************/
9-
package org.cryptomator.cryptofs.fh;
9+
package org.cryptomator.cryptofs.ch;
1010

1111
import org.hamcrest.MatcherAssert;
12+
import org.junit.jupiter.api.Assertions;
1213
import org.junit.jupiter.api.Nested;
1314
import org.junit.jupiter.api.Test;
1415

1516
import java.nio.ByteBuffer;
17+
import java.util.Arrays;
1618

1719
import static org.hamcrest.CoreMatchers.is;
1820

@@ -80,4 +82,44 @@ public void testCopyToWithLessRemainingInTargetThanInSource() {
8082

8183
}
8284

85+
@Nested
86+
public class RepeatingZeros {
87+
88+
@Test
89+
public void testRemainingCombinesZerosWithBuffer() {
90+
ByteBuffer buffer = ByteBuffer.wrap(new byte[]{(byte) 0xFF});
91+
ByteSource inTest = ByteSource.repeatingZeroes(41).followedBy(buffer);
92+
93+
Assertions.assertTrue(inTest.hasRemaining());
94+
Assertions.assertEquals(42, inTest.remaining());
95+
}
96+
97+
@Test
98+
public void testCopyToWritesZeros() {
99+
ByteBuffer buffer = ByteBuffer.wrap(new byte[]{(byte) 0x77});
100+
ByteSource inTest = ByteSource.repeatingZeroes(41).followedBy(buffer);
101+
byte[] target = new byte[50];
102+
Arrays.fill(target, (byte) 0xFF); // pre-fill target to check whether data gets zero'ed
103+
104+
inTest.copyTo(ByteBuffer.wrap(target));
105+
106+
Assertions.assertArrayEquals(new byte[41], Arrays.copyOf(target, 41));
107+
Assertions.assertEquals(0x77, target[41]);
108+
}
109+
110+
@Test
111+
public void testCopyToWritesLotsOfZeros() {
112+
ByteBuffer buffer = ByteBuffer.wrap(new byte[]{(byte) 0x77});
113+
ByteSource inTest = ByteSource.repeatingZeroes(9999).followedBy(buffer);
114+
byte[] target = new byte[10_000];
115+
Arrays.fill(target, (byte) 0xFF); // pre-fill target to check whether data gets zero'ed
116+
117+
inTest.copyTo(ByteBuffer.wrap(target));
118+
119+
Assertions.assertArrayEquals(new byte[9999], Arrays.copyOf(target, 9999));
120+
Assertions.assertEquals(0x77, target[9999]);
121+
}
122+
123+
}
124+
83125
}

0 commit comments

Comments
 (0)