This is an automated email from the ASF dual-hosted git repository. bschuchardt pushed a commit to branch feature/GEODE-7540b in repository https://gitbox.apache.org/repos/asf/geode.git
commit 04cd67d21fcef6747d53859d2d32cff89b7a5920 Author: Bruce Schuchardt <[email protected]> AuthorDate: Fri Nov 15 15:31:10 2019 -0800 GEODE-7450 SSL peerAppDataBuffer expansion needs work ensure that the buffer capacity always increases after an overflow status is returned. --- .../apache/geode/internal/net/NioSslEngine.java | 1 + .../geode/internal/net/NioSslEngineTest.java | 54 ++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java b/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java index 7756dcf..9d67594 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java +++ b/geode-core/src/main/java/org/apache/geode/internal/net/NioSslEngine.java @@ -275,6 +275,7 @@ public class NioSslEngine implements NioFilter { // buffer overflow expand and try again - double the available decryption space int newCapacity = (peerAppData.capacity() - peerAppData.position()) * 2 + peerAppData.position(); + newCapacity = Math.max(newCapacity, peerAppData.capacity() / 2 * 3); peerAppData = bufferPool.expandWriteBufferIfNeeded(TRACKED_RECEIVER, peerAppData, newCapacity); peerAppData.limit(peerAppData.capacity()); diff --git a/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java b/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java index 46551b7..1d5e4a6 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java +++ b/geode-core/src/test/java/org/apache/geode/internal/net/NioSslEngineTest.java @@ -477,6 +477,53 @@ public class NioSslEngineTest { } + /** + * This tests the case where a message header has been read and part of a message has been + * read, but the decoded buffer is too small to hold all of the message. In this case + * the buffer is completely full and should only take one overflow response to resolve + * the problem. + */ + @Test + public void readAtLeastUsingSmallAppBufferAtWriteLimit() throws Exception { + final int amountToRead = 150; + final int individualRead = 150; + + int initialUnwrappedBufferSize = 100; + final int preexistingBytes = initialUnwrappedBufferSize - 7; + ByteBuffer wrappedBuffer = ByteBuffer.allocate(1000); + SocketChannel mockChannel = mock(SocketChannel.class); + + // force buffer expansion by making a small decoded buffer appear near to being full + ByteBuffer unwrappedBuffer = ByteBuffer.allocate(initialUnwrappedBufferSize); + unwrappedBuffer.position(7).limit(preexistingBytes + 7); // 7 bytes of message header - ignored + nioSslEngine.peerAppData = unwrappedBuffer; + + // simulate some socket reads + when(mockChannel.read(any(ByteBuffer.class))).thenAnswer(new Answer<Integer>() { + @Override + public Integer answer(InvocationOnMock invocation) throws Throwable { + ByteBuffer buffer = invocation.getArgument(0); + buffer.position(buffer.position() + individualRead); + return individualRead; + } + }); + + TestSSLEngine testSSLEngine = new TestSSLEngine(); + testSSLEngine.addReturnResult( + new SSLEngineResult(BUFFER_OVERFLOW, NEED_UNWRAP, 0, 0), + new SSLEngineResult(BUFFER_OVERFLOW, NEED_UNWRAP, 0, 0), + new SSLEngineResult(BUFFER_OVERFLOW, NEED_UNWRAP, 0, 0), + new SSLEngineResult(OK, NEED_UNWRAP, 0, 0)); + nioSslEngine.engine = testSSLEngine; + + ByteBuffer data = nioSslEngine.readAtLeast(mockChannel, amountToRead, wrappedBuffer); + verify(mockChannel, times(1)).read(isA(ByteBuffer.class)); + assertThat(data.position()).isEqualTo(0); + assertThat(data.limit()) + .isEqualTo(individualRead * testSSLEngine.getNumberOfUnwraps() + preexistingBytes); + } + + // TestSSLEngine holds a stack of SSLEngineResults and always copies the // input buffer to the output buffer byte-for-byte in wrap() and unwrap() operations. // We use it in some tests where we need the byte-copying behavior because it's @@ -485,6 +532,8 @@ public class NioSslEngineTest { private List<SSLEngineResult> returnResults = new ArrayList<>(); + private int numberOfUnwrapsPerformed; + private SSLEngineResult nextResult() { SSLEngineResult result = returnResults.remove(0); if (returnResults.isEmpty()) { @@ -493,6 +542,10 @@ public class NioSslEngineTest { return result; } + public int getNumberOfUnwraps() { + return numberOfUnwrapsPerformed; + } + @Override public SSLEngineResult wrap(ByteBuffer[] sources, int i, int i1, ByteBuffer destination) { for (ByteBuffer source : sources) { @@ -507,6 +560,7 @@ public class NioSslEngineTest { if (sslEngineResult.getStatus() != BUFFER_UNDERFLOW && sslEngineResult.getStatus() != BUFFER_OVERFLOW) { destinations[0].put(source); + numberOfUnwrapsPerformed++; } return sslEngineResult; }
