geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bschucha...@apache.org
Subject [geode] branch develop updated: GEODE-7450 SSL peerAppDataBuffer expansion needs work (#4336)
Date Mon, 18 Nov 2019 16:04:04 GMT
This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new f46b492  GEODE-7450 SSL peerAppDataBuffer expansion needs work (#4336)
f46b492 is described below

commit f46b492cf09ed2626646b2da06a6f76e4a23794e
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
AuthorDate: Mon Nov 18 08:03:09 2019 -0800

    GEODE-7450 SSL peerAppDataBuffer expansion needs work (#4336)
    
    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;
     }


Mime
View raw message