Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should opening a second channel with TRUNCATE_EXISTING use a new file header? #160

Closed
overheadhunter opened this issue Mar 21, 2023 · 1 comment · Fixed by #161
Closed

Comments

@overheadhunter
Copy link
Member

On CI (not reproducible on all setups, though) testTruncateExistingWhileStillOpen fails.

Due to TRUNCATE_EXISTING flag being passed through to the ciphertext file channel, the ciphertext file size is reset to zero as soon as the method is invoked, which causes a new header to be created:

public synchronized FileChannel newFileChannel(EffectiveOpenOptions options, FileAttribute<?>... attrs) throws IOException {
Path path = currentFilePath.get();
if (options.truncateExisting()) {
chunkCache.invalidateAll();
}
FileChannel ciphertextFileChannel = null;
CleartextFileChannel cleartextFileChannel = null;
try {
ciphertextFileChannel = path.getFileSystem().provider().newFileChannel(path, options.createOpenOptionsForEncryptedFile(), attrs);
final FileHeader header;
final boolean isNewHeader;
if (ciphertextFileChannel.size() == 0l) {
header = headerHolder.createNew();
isNewHeader = true;
} else {
header = headerHolder.loadExisting(ciphertextFileChannel);
isNewHeader = false;
}

I'm currently unsure, if this is desirable behaviour (and the test is broken) or the existing header should really be kept.

@overheadhunter
Copy link
Member Author

overheadhunter commented Mar 22, 2023

As already specified in #48: Yes TRUNCATE_EXISTING should generate a new header. However FileHeaderHolder will only generate a header once due to this atomic CAS operation:

public FileHeader createNew() {
LOG.trace("Generating file header for {}", path.get());
FileHeader newHeader = cryptor.fileHeaderCryptor().create();
if (header.compareAndSet(null, newHeader)) {
return newHeader;
} else {
return header.get();
}
}

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

Successfully merging a pull request may close this issue.

1 participant