From 57302b30409ee078c8a8aa0e3f9dfcf4f906f0c4 Mon Sep 17 00:00:00 2001 From: Joachim Draeger Date: Thu, 15 Jun 2017 14:14:41 +0100 Subject: [PATCH 1/7] Moved SocketAccess.doPrivileged up the stack to DefaultS3OutputStream in repository-S3 plugin to avoid SecurityException by Streams.copy(). A plugin is only allowed to use its own jars when performing privileged operations. The S3 client might open a new Socket on close(). #25192 --- .../repositories/s3/DefaultS3OutputStream.java | 7 +++++++ .../org/elasticsearch/repositories/s3/S3BlobContainer.java | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/DefaultS3OutputStream.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/DefaultS3OutputStream.java index e80f07ac55eb2..9102ae0ff290d 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/DefaultS3OutputStream.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/DefaultS3OutputStream.java @@ -78,6 +78,13 @@ class DefaultS3OutputStream extends S3OutputStream { @Override public void flush(byte[] bytes, int off, int len, boolean closing) throws IOException { + SocketAccess.doPrivilegedIOException(() -> { + flushPriveleged(bytes, off, len, closing); + return null; + }); + } + + private void flushPriveleged(byte[] bytes, int off, int len, boolean closing) throws IOException { if (len > MULTIPART_MAX_SIZE.getBytes()) { throw new IOException("Unable to upload files larger than " + MULTIPART_MAX_SIZE + " to Amazon S3"); } diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java index 8af31377f875b..f49f4b348f02b 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java @@ -92,7 +92,7 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize) t throw new FileAlreadyExistsException("blob [" + blobName + "] already exists, cannot overwrite"); } try (OutputStream stream = createOutput(blobName)) { - SocketAccess.doPrivilegedIOException(() -> Streams.copy(inputStream, stream)); + Streams.copy(inputStream, stream); } } From e924786ecbfaa584084929f0f4b1d34bbec50df6 Mon Sep 17 00:00:00 2001 From: Joachim Draeger Date: Thu, 15 Jun 2017 18:16:03 +0100 Subject: [PATCH 2/7] PoC for checking SocketPetmissions in repository-s3 plugin --- .../repositories/s3/MockAmazonS3.java | 18 ++++++++++++ .../s3/S3BlobStoreContainerTests.java | 28 +++++++++++++++++++ .../bootstrap/BootstrapForTesting.java | 2 +- 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java index 3f6ce26232bfe..01738638cd098 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java @@ -40,6 +40,7 @@ import java.io.IOException; import java.io.InputStream; +import java.net.Socket; import java.security.DigestInputStream; import java.util.ArrayList; import java.util.List; @@ -54,6 +55,17 @@ class MockAmazonS3 extends AbstractAmazonS3 { // length of the input data is 100 bytes private byte[] byteCounter = new byte[100]; + + private void openSocket() { + try { + Socket socket = new Socket("localhost", 9200); + socket.close(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override public boolean doesBucketExist(String bucket) { return true; @@ -63,6 +75,7 @@ public boolean doesBucketExist(String bucket) { public ObjectMetadata getObjectMetadata( GetObjectMetadataRequest getObjectMetadataRequest) throws AmazonClientException, AmazonServiceException { + openSocket(); String blobName = getObjectMetadataRequest.getKey(); if (!blobs.containsKey(blobName)) { @@ -75,6 +88,7 @@ public ObjectMetadata getObjectMetadata( @Override public PutObjectResult putObject(PutObjectRequest putObjectRequest) throws AmazonClientException, AmazonServiceException { + openSocket(); String blobName = putObjectRequest.getKey(); DigestInputStream stream = (DigestInputStream) putObjectRequest.getInputStream(); @@ -95,6 +109,7 @@ public PutObjectResult putObject(PutObjectRequest putObjectRequest) @Override public S3Object getObject(GetObjectRequest getObjectRequest) throws AmazonClientException, AmazonServiceException { + openSocket(); // in ESBlobStoreContainerTestCase.java, the prefix is empty, // so the key and blobName are equivalent to each other String blobName = getObjectRequest.getKey(); @@ -114,6 +129,7 @@ public S3Object getObject(GetObjectRequest getObjectRequest) @Override public ObjectListing listObjects(ListObjectsRequest listObjectsRequest) throws AmazonClientException, AmazonServiceException { + openSocket(); MockObjectListing list = new MockObjectListing(); list.setTruncated(false); @@ -147,6 +163,7 @@ public ObjectListing listObjects(ListObjectsRequest listObjectsRequest) @Override public CopyObjectResult copyObject(CopyObjectRequest copyObjectRequest) throws AmazonClientException, AmazonServiceException { + openSocket(); String sourceBlobName = copyObjectRequest.getSourceKey(); String targetBlobName = copyObjectRequest.getDestinationKey(); @@ -167,6 +184,7 @@ public CopyObjectResult copyObject(CopyObjectRequest copyObjectRequest) @Override public void deleteObject(DeleteObjectRequest deleteObjectRequest) throws AmazonClientException, AmazonServiceException { + openSocket(); String blobName = deleteObjectRequest.getKey(); if (!blobs.containsKey(blobName)) { diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java index e43a9011200a2..8dada5bb61efd 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java @@ -24,11 +24,33 @@ import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.repositories.ESBlobStoreContainerTestCase; +import org.junit.AfterClass; +import org.junit.BeforeClass; import java.io.IOException; +import java.net.ServerSocket; import java.util.Locale; public class S3BlobStoreContainerTests extends ESBlobStoreContainerTestCase { + + private static ServerSocket socket; + + @BeforeClass + public static void openPort() throws IOException { + System.out.println("openPort()"); + socket = new ServerSocket(9200); + new Thread(() -> { + while (!socket.isClosed()) { + try { + socket.accept(); + System.out.println("accept!"); + } catch (IOException e) { + System.out.println(e); + } + } + }).start(); + } + protected BlobStore newBlobStore() throws IOException { MockAmazonS3 client = new MockAmazonS3(); String bucket = randomAlphaOfLength(randomIntBetween(1, 10)).toLowerCase(Locale.ROOT); @@ -36,4 +58,10 @@ protected BlobStore newBlobStore() throws IOException { return new S3BlobStore(Settings.EMPTY, client, bucket, false, new ByteSizeValue(10, ByteSizeUnit.MB), "public-read-write", "standard"); } + + @AfterClass + public static void closePort() throws IOException { + System.out.println("closePort()"); + socket.close(); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java index 62bf89f6d3116..f52eaf47e96e7 100644 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java +++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java @@ -129,7 +129,7 @@ public class BootstrapForTesting { // ... but tests are messy. like file permissions, just let them live in a fantasy for now. // TODO: cut over all tests to bind to ephemeral ports perms.add(new SocketPermission("localhost:1024-", "listen,resolve")); - + perms.add(new SocketPermission("localhost:1024-", "accept,resolve")); // read test-framework permissions final Policy testFramework = Security.readPolicy(Bootstrap.class.getResource("test-framework.policy"), JarHell.parseClassPath()); final Policy esPolicy = new ESPolicy(perms, getPluginPermissions(), true); From a136d9309da96c2892a5e569ce1e310dee94ccd1 Mon Sep 17 00:00:00 2001 From: Joachim Draeger Date: Wed, 21 Jun 2017 10:27:18 +0100 Subject: [PATCH 3/7] Fix spelling --- .../elasticsearch/repositories/s3/DefaultS3OutputStream.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/DefaultS3OutputStream.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/DefaultS3OutputStream.java index 9102ae0ff290d..c20259fe9bffa 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/DefaultS3OutputStream.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/DefaultS3OutputStream.java @@ -79,12 +79,12 @@ class DefaultS3OutputStream extends S3OutputStream { @Override public void flush(byte[] bytes, int off, int len, boolean closing) throws IOException { SocketAccess.doPrivilegedIOException(() -> { - flushPriveleged(bytes, off, len, closing); + flushPrivileged(bytes, off, len, closing); return null; }); } - private void flushPriveleged(byte[] bytes, int off, int len, boolean closing) throws IOException { + private void flushPrivileged(byte[] bytes, int off, int len, boolean closing) throws IOException { if (len > MULTIPART_MAX_SIZE.getBytes()) { throw new IOException("Unable to upload files larger than " + MULTIPART_MAX_SIZE + " to Amazon S3"); } From e47d36088be87f40162e11a5b78340d943c5c7f5 Mon Sep 17 00:00:00 2001 From: Joachim Draeger Date: Wed, 21 Jun 2017 17:27:53 +0100 Subject: [PATCH 4/7] fix api checks --- .../elasticsearch/repositories/s3/MockAmazonS3.java | 3 ++- .../repositories/s3/S3BlobStoreContainerTests.java | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java index 01738638cd098..1b2b6c9f45c2d 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java @@ -40,6 +40,7 @@ import java.io.IOException; import java.io.InputStream; +import java.net.InetAddress; import java.net.Socket; import java.security.DigestInputStream; import java.util.ArrayList; @@ -58,7 +59,7 @@ class MockAmazonS3 extends AbstractAmazonS3 { private void openSocket() { try { - Socket socket = new Socket("localhost", 9200); + Socket socket = new Socket(InetAddress.getByName("127.0.0.1"), 9200); socket.close(); } catch (IOException e) { throw new RuntimeException(e); diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java index 8dada5bb61efd..0798c125031ac 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java @@ -19,7 +19,10 @@ package org.elasticsearch.repositories.s3; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.Logger; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; @@ -28,24 +31,25 @@ import org.junit.BeforeClass; import java.io.IOException; +import java.net.InetAddress; import java.net.ServerSocket; import java.util.Locale; public class S3BlobStoreContainerTests extends ESBlobStoreContainerTestCase { + private static final Logger logger = Loggers.getLogger(S3BlobStoreContainerTests.class); + private static ServerSocket socket; @BeforeClass public static void openPort() throws IOException { - System.out.println("openPort()"); - socket = new ServerSocket(9200); + socket = new ServerSocket(9200, 50, InetAddress.getByName("127.0.0.1")); new Thread(() -> { while (!socket.isClosed()) { try { socket.accept(); - System.out.println("accept!"); } catch (IOException e) { - System.out.println(e); + logger.log(Level.ERROR, e); } } }).start(); @@ -61,7 +65,6 @@ protected BlobStore newBlobStore() throws IOException { @AfterClass public static void closePort() throws IOException { - System.out.println("closePort()"); socket.close(); } } From f9fe3b94ff90e10feefcdd1b625c3e875fb5a7ae Mon Sep 17 00:00:00 2001 From: Joachim Draeger Date: Wed, 21 Jun 2017 18:10:20 +0100 Subject: [PATCH 5/7] Use MockSocket to simulate S3 connections --- .../repositories/s3/MockAmazonS3.java | 23 ++++++++++++------- .../s3/S3BlobStoreContainerTests.java | 10 ++++---- .../bootstrap/BootstrapForTesting.java | 2 +- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java index 1b2b6c9f45c2d..4a664e8d0aebf 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java @@ -50,6 +50,8 @@ class MockAmazonS3 extends AbstractAmazonS3 { + private final int mockSocketPort; + private Map blobs = new ConcurrentHashMap<>(); // in ESBlobStoreContainerTestCase.java, the maximum @@ -57,9 +59,14 @@ class MockAmazonS3 extends AbstractAmazonS3 { private byte[] byteCounter = new byte[100]; - private void openSocket() { + MockAmazonS3(int mockSocketPort) { + this.mockSocketPort = mockSocketPort; + } + + // simulate a socket connection to check that SocketAccess is used correctly + private void simulateS3SocketConnection() { try { - Socket socket = new Socket(InetAddress.getByName("127.0.0.1"), 9200); + Socket socket = new Socket(InetAddress.getByName("127.0.0.1"), mockSocketPort); socket.close(); } catch (IOException e) { throw new RuntimeException(e); @@ -76,7 +83,7 @@ public boolean doesBucketExist(String bucket) { public ObjectMetadata getObjectMetadata( GetObjectMetadataRequest getObjectMetadataRequest) throws AmazonClientException, AmazonServiceException { - openSocket(); + simulateS3SocketConnection(); String blobName = getObjectMetadataRequest.getKey(); if (!blobs.containsKey(blobName)) { @@ -89,7 +96,7 @@ public ObjectMetadata getObjectMetadata( @Override public PutObjectResult putObject(PutObjectRequest putObjectRequest) throws AmazonClientException, AmazonServiceException { - openSocket(); + simulateS3SocketConnection(); String blobName = putObjectRequest.getKey(); DigestInputStream stream = (DigestInputStream) putObjectRequest.getInputStream(); @@ -110,7 +117,7 @@ public PutObjectResult putObject(PutObjectRequest putObjectRequest) @Override public S3Object getObject(GetObjectRequest getObjectRequest) throws AmazonClientException, AmazonServiceException { - openSocket(); + simulateS3SocketConnection(); // in ESBlobStoreContainerTestCase.java, the prefix is empty, // so the key and blobName are equivalent to each other String blobName = getObjectRequest.getKey(); @@ -130,7 +137,7 @@ public S3Object getObject(GetObjectRequest getObjectRequest) @Override public ObjectListing listObjects(ListObjectsRequest listObjectsRequest) throws AmazonClientException, AmazonServiceException { - openSocket(); + simulateS3SocketConnection(); MockObjectListing list = new MockObjectListing(); list.setTruncated(false); @@ -164,7 +171,7 @@ public ObjectListing listObjects(ListObjectsRequest listObjectsRequest) @Override public CopyObjectResult copyObject(CopyObjectRequest copyObjectRequest) throws AmazonClientException, AmazonServiceException { - openSocket(); + simulateS3SocketConnection(); String sourceBlobName = copyObjectRequest.getSourceKey(); String targetBlobName = copyObjectRequest.getDestinationKey(); @@ -185,7 +192,7 @@ public CopyObjectResult copyObject(CopyObjectRequest copyObjectRequest) @Override public void deleteObject(DeleteObjectRequest deleteObjectRequest) throws AmazonClientException, AmazonServiceException { - openSocket(); + simulateS3SocketConnection(); String blobName = deleteObjectRequest.getKey(); if (!blobs.containsKey(blobName)) { diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java index 0798c125031ac..a11bd90bc8070 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; +import org.elasticsearch.mocksocket.MockServerSocket; import org.elasticsearch.repositories.ESBlobStoreContainerTestCase; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -41,9 +42,10 @@ public class S3BlobStoreContainerTests extends ESBlobStoreContainerTestCase { private static ServerSocket socket; + // Opens a MockSocket to simulate connections to S3 checking that SocketPermissions are set up correctly @BeforeClass - public static void openPort() throws IOException { - socket = new ServerSocket(9200, 50, InetAddress.getByName("127.0.0.1")); + public static void openMockSocket() throws IOException { + socket = new MockServerSocket(0, 50, InetAddress.getByName("127.0.0.1")); new Thread(() -> { while (!socket.isClosed()) { try { @@ -56,7 +58,7 @@ public static void openPort() throws IOException { } protected BlobStore newBlobStore() throws IOException { - MockAmazonS3 client = new MockAmazonS3(); + MockAmazonS3 client = new MockAmazonS3(socket.getLocalPort()); String bucket = randomAlphaOfLength(randomIntBetween(1, 10)).toLowerCase(Locale.ROOT); return new S3BlobStore(Settings.EMPTY, client, bucket, false, @@ -64,7 +66,7 @@ protected BlobStore newBlobStore() throws IOException { } @AfterClass - public static void closePort() throws IOException { + public static void closeMockSocket() throws IOException { socket.close(); } } diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java index f52eaf47e96e7..62bf89f6d3116 100644 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java +++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java @@ -129,7 +129,7 @@ public class BootstrapForTesting { // ... but tests are messy. like file permissions, just let them live in a fantasy for now. // TODO: cut over all tests to bind to ephemeral ports perms.add(new SocketPermission("localhost:1024-", "listen,resolve")); - perms.add(new SocketPermission("localhost:1024-", "accept,resolve")); + // read test-framework permissions final Policy testFramework = Security.readPolicy(Bootstrap.class.getResource("test-framework.policy"), JarHell.parseClassPath()); final Policy esPolicy = new ESPolicy(perms, getPluginPermissions(), true); From bd47ee9337b030cae08cf3017adb669b4141680c Mon Sep 17 00:00:00 2001 From: Joachim Draeger Date: Mon, 3 Jul 2017 17:17:41 +0100 Subject: [PATCH 6/7] More documentation to clarify the role of using a MockSocket in S3BlobStoreContainerTests and further improvements. --- .../repositories/s3/MockAmazonS3.java | 18 ++++++++---- .../s3/S3BlobStoreContainerTests.java | 28 ++++++++++++------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java index 4a664e8d0aebf..bfd7751670b7b 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java @@ -37,9 +37,11 @@ import com.amazonaws.services.s3.model.S3ObjectInputStream; import com.amazonaws.services.s3.model.S3ObjectSummary; import com.amazonaws.util.Base64; +import org.junit.Assert; import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; import java.net.InetAddress; import java.net.Socket; import java.security.DigestInputStream; @@ -63,13 +65,19 @@ class MockAmazonS3 extends AbstractAmazonS3 { this.mockSocketPort = mockSocketPort; } - // simulate a socket connection to check that SocketAccess is used correctly + // Simulate a socket connection to check that SocketAccess.doPrivileged() is used correctly. + // Any method of AmazonS3 might potentially open a socket to the S3 service. Firstly, a call + // to any method of AmazonS3 has to be wrapped by SocketAccess.doPrivileged(). + // Secondly, each method on the stack from doPrivileged to opening the socket has to be + // located in a jar that is provided by the plugin. + // Thirdly, a SocketPermission has to be configured in plugin-security.policy. + // By opening a socket in each method of MockAmazonS3 it is ensured that in production AmazonS3 + // is able to to open a socket to the S3 Service without causing a SecurityException private void simulateS3SocketConnection() { - try { - Socket socket = new Socket(InetAddress.getByName("127.0.0.1"), mockSocketPort); - socket.close(); + try (Socket socket = new Socket(InetAddress.getByName("127.0.0.1"), mockSocketPort)) { + Assert.assertTrue(socket.isConnected()); // NOOP to keep static analysis happy } catch (IOException e) { - throw new RuntimeException(e); + throw new UncheckedIOException(e); } } diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java index a11bd90bc8070..ea18c784d7ead 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java @@ -40,25 +40,30 @@ public class S3BlobStoreContainerTests extends ESBlobStoreContainerTestCase { private static final Logger logger = Loggers.getLogger(S3BlobStoreContainerTests.class); - private static ServerSocket socket; + private static ServerSocket mockS3ServerSocket; - // Opens a MockSocket to simulate connections to S3 checking that SocketPermissions are set up correctly + private static Thread mockS3AcceptorThread; + + // Opens a MockSocket to simulate connections to S3 checking that SocketPermissions are set up correctly. + // See MockAmazonS3.simulateS3SocketConnection. @BeforeClass public static void openMockSocket() throws IOException { - socket = new MockServerSocket(0, 50, InetAddress.getByName("127.0.0.1")); - new Thread(() -> { - while (!socket.isClosed()) { + mockS3ServerSocket = new MockServerSocket(0, 50, InetAddress.getByName("127.0.0.1")); + mockS3AcceptorThread = new Thread(() -> { + while (!mockS3ServerSocket.isClosed()) { try { - socket.accept(); + // Accept connections from MockAmazonS3. + mockS3ServerSocket.accept(); } catch (IOException e) { logger.log(Level.ERROR, e); } } - }).start(); + }); + mockS3AcceptorThread.start(); } protected BlobStore newBlobStore() throws IOException { - MockAmazonS3 client = new MockAmazonS3(socket.getLocalPort()); + MockAmazonS3 client = new MockAmazonS3(mockS3ServerSocket.getLocalPort()); String bucket = randomAlphaOfLength(randomIntBetween(1, 10)).toLowerCase(Locale.ROOT); return new S3BlobStore(Settings.EMPTY, client, bucket, false, @@ -66,7 +71,10 @@ protected BlobStore newBlobStore() throws IOException { } @AfterClass - public static void closeMockSocket() throws IOException { - socket.close(); + public static void closeMockSocket() throws IOException, InterruptedException { + mockS3ServerSocket.close(); + mockS3AcceptorThread.join(); + mockS3AcceptorThread = null; + mockS3ServerSocket = null; } } From 90ea0ad4566d310b315d1f907fdffaeeb400836a Mon Sep 17 00:00:00 2001 From: Joachim Draeger Date: Tue, 4 Jul 2017 12:02:01 +0100 Subject: [PATCH 7/7] Removed obsolete IOException in test, static import of assertTrue --- .../java/org/elasticsearch/repositories/s3/MockAmazonS3.java | 5 +++-- .../repositories/s3/S3BlobStoreContainerTests.java | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java index bfd7751670b7b..d3c76cd602100 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java @@ -37,7 +37,6 @@ import com.amazonaws.services.s3.model.S3ObjectInputStream; import com.amazonaws.services.s3.model.S3ObjectSummary; import com.amazonaws.util.Base64; -import org.junit.Assert; import java.io.IOException; import java.io.InputStream; @@ -50,6 +49,8 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import static org.junit.Assert.assertTrue; + class MockAmazonS3 extends AbstractAmazonS3 { private final int mockSocketPort; @@ -75,7 +76,7 @@ class MockAmazonS3 extends AbstractAmazonS3 { // is able to to open a socket to the S3 Service without causing a SecurityException private void simulateS3SocketConnection() { try (Socket socket = new Socket(InetAddress.getByName("127.0.0.1"), mockSocketPort)) { - Assert.assertTrue(socket.isConnected()); // NOOP to keep static analysis happy + assertTrue(socket.isConnected()); // NOOP to keep static analysis happy } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java index ea18c784d7ead..45ffac30aa7fb 100644 --- a/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java +++ b/plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java @@ -55,7 +55,6 @@ public static void openMockSocket() throws IOException { // Accept connections from MockAmazonS3. mockS3ServerSocket.accept(); } catch (IOException e) { - logger.log(Level.ERROR, e); } } });