dsmiley commented on code in PR #112: URL: https://github.com/apache/solr-sandbox/pull/112#discussion_r1806773936
########## encryption/src/main/java/org/apache/solr/encryption/EncryptionTransactionLog.java: ########## @@ -140,28 +139,33 @@ private static void writeBEInt(OutputStream outputStream, int i) throws IOExcept * * @return The key reference number as a string; or null if the log is not encrypted. */ - static String readEncryptionHeader(FileChannel channel, ByteBuffer readBuffer) throws IOException { - long position = channel.position(); - if (position != 0) { - channel.position(0); - } - int magic = readBEInt(channel, readBuffer); + static String readEncryptionHeader(FileChannel channel) throws IOException { + int magic = readBEInt(channel, 0L, false); String keyRef = null; if (magic == ENCRYPTION_MAGIC) { // This file is encrypted. // Read the key reference that follows. - keyRef = Integer.toString(readBEInt(channel, readBuffer)); + keyRef = Integer.toString(readBEInt(channel, 4L, true)); } - channel.position(position); return keyRef; } - private static int readBEInt(ReadableByteChannel channel, ByteBuffer readBuffer) throws IOException { - readBuffer.clear(); - int n = channel.read(readBuffer); - if (n != 4) { - throw new EOFException(); + private static int readBEInt(FileChannel channel, long position, boolean requireAllBytes) throws IOException { + ByteBuffer readBuffer = ByteBuffer.allocate(4); + // Read 4 bytes. + int bytesRead = channel.read(readBuffer, position); + if (bytesRead < 4) { + if (requireAllBytes) { + throw new EOFException( + bytesRead == -1 || bytesRead == 0 + ? "Header is empty; no data read." + : "Incomplete header; expected 4 bytes, but only read " + bytesRead + " bytes."); + } else { + // If not requiring all bytes, just return 0. + return 0; + } Review Comment: a minor opinion I have is that we shouldn't support "requireAllBytes" because it'd be weird for them to not be there. The caller, readEncryptionHeader, could catch EOFException and return an empty string if we think that's sensible semantically (or propagate that exception maybe). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org