Browse Source

Injected response errors in Azure repository tests should have a body (#47176)

The Azure SDK client expects server errors to have a body, 
something that looks like:

<?xml version="1.0" encoding="utf-8"?>  
<Error>  
  <Code>string-value</Code>  
  <Message>string-value</Message>  
</Error> 

I've forgot to add such errors in Azure tests and that triggers 
some NPE in the client like the one reported in #47120.

Closes #47120
Tanguy Leroux 6 years ago
parent
commit
c5b9cdac81

+ 6 - 10
plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java

@@ -24,7 +24,6 @@ import com.microsoft.azure.storage.RetryPolicyFactory;
 import com.microsoft.azure.storage.blob.BlobRequestOptions;
 import com.sun.net.httpserver.HttpExchange;
 import com.sun.net.httpserver.HttpServer;
-import org.apache.http.HttpStatus;
 import org.elasticsearch.cluster.metadata.RepositoryMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.SuppressForbidden;
@@ -164,7 +163,7 @@ public class AzureBlobContainerRetriesTests extends ESTestCase {
                     exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
                     exchange.getResponseHeaders().add("x-ms-blob-content-length", String.valueOf(bytes.length));
                     exchange.getResponseHeaders().add("x-ms-blob-type", "blockblob");
-                    exchange.sendResponseHeaders(HttpStatus.SC_OK, -1);
+                    exchange.sendResponseHeaders(RestStatus.OK.getStatus(), -1);
                     exchange.close();
                     return;
                 }
@@ -176,15 +175,14 @@ public class AzureBlobContainerRetriesTests extends ESTestCase {
                     exchange.getResponseHeaders().add("Content-Type", "application/octet-stream");
                     exchange.getResponseHeaders().add("x-ms-blob-content-length", String.valueOf(length));
                     exchange.getResponseHeaders().add("x-ms-blob-type", "blockblob");
-                    exchange.sendResponseHeaders(HttpStatus.SC_OK, length);
+                    exchange.sendResponseHeaders(RestStatus.OK.getStatus(), length);
                     exchange.getResponseBody().write(bytes, rangeStart, length);
                     exchange.close();
                     return;
                 }
             }
             if (randomBoolean()) {
-                exchange.sendResponseHeaders(randomFrom(HttpStatus.SC_INTERNAL_SERVER_ERROR, HttpStatus.SC_BAD_GATEWAY,
-                                                        HttpStatus.SC_SERVICE_UNAVAILABLE, HttpStatus.SC_GATEWAY_TIMEOUT), -1);
+                TestUtils.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
             }
             exchange.close();
         });
@@ -209,7 +207,7 @@ public class AzureBlobContainerRetriesTests extends ESTestCase {
                     if (Objects.deepEquals(bytes, BytesReference.toBytes(body))) {
                         exchange.sendResponseHeaders(RestStatus.CREATED.getStatus(), -1);
                     } else {
-                        exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, -1);
+                        TestUtils.sendError(exchange, RestStatus.BAD_REQUEST);
                     }
                     exchange.close();
                     return;
@@ -220,8 +218,7 @@ public class AzureBlobContainerRetriesTests extends ESTestCase {
                         Streams.readFully(exchange.getRequestBody(), new byte[randomIntBetween(1, Math.max(1, bytes.length - 1))]);
                     } else {
                         Streams.readFully(exchange.getRequestBody());
-                        exchange.sendResponseHeaders(randomFrom(HttpStatus.SC_INTERNAL_SERVER_ERROR, HttpStatus.SC_BAD_GATEWAY,
-                                                                HttpStatus.SC_SERVICE_UNAVAILABLE, HttpStatus.SC_GATEWAY_TIMEOUT), -1);
+                        TestUtils.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
                     }
                 }
                 exchange.close();
@@ -283,8 +280,7 @@ public class AzureBlobContainerRetriesTests extends ESTestCase {
 
             if (randomBoolean()) {
                 Streams.readFully(exchange.getRequestBody());
-                exchange.sendResponseHeaders(randomFrom(HttpStatus.SC_INTERNAL_SERVER_ERROR, HttpStatus.SC_BAD_GATEWAY,
-                                                        HttpStatus.SC_SERVICE_UNAVAILABLE, HttpStatus.SC_GATEWAY_TIMEOUT), -1);
+                TestUtils.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
             }
             exchange.close();
         });

+ 10 - 3
plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java

@@ -171,7 +171,7 @@ public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryInteg
                 } else if (Regex.simpleMatch("HEAD /container/*", request)) {
                     final BytesReference blob = blobs.get(exchange.getRequestURI().getPath());
                     if (blob == null) {
-                        exchange.sendResponseHeaders(RestStatus.NOT_FOUND.getStatus(), -1);
+                        TestUtils.sendError(exchange, RestStatus.NOT_FOUND);
                         return;
                     }
                     exchange.getResponseHeaders().add("x-ms-blob-content-length", String.valueOf(blob.length()));
@@ -181,7 +181,7 @@ public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryInteg
                 } else if (Regex.simpleMatch("GET /container/*", request)) {
                     final BytesReference blob = blobs.get(exchange.getRequestURI().getPath());
                     if (blob == null) {
-                        exchange.sendResponseHeaders(RestStatus.NOT_FOUND.getStatus(), -1);
+                        TestUtils.sendError(exchange, RestStatus.NOT_FOUND);
                         return;
                     }
 
@@ -228,7 +228,7 @@ public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryInteg
                     exchange.getResponseBody().write(response);
 
                 } else {
-                    exchange.sendResponseHeaders(RestStatus.BAD_REQUEST.getStatus(), -1);
+                    TestUtils.sendError(exchange, RestStatus.BAD_REQUEST);
                 }
             } finally {
                 exchange.close();
@@ -249,6 +249,13 @@ public class AzureBlobStoreRepositoryTests extends ESMockAPIBasedRepositoryInteg
             super(delegate, maxErrorsPerRequest);
         }
 
+        @Override
+        protected void handleAsError(final HttpExchange exchange) throws IOException {
+            Streams.readFully(exchange.getRequestBody());
+            TestUtils.sendError(exchange, randomFrom(RestStatus.INTERNAL_SERVER_ERROR, RestStatus.SERVICE_UNAVAILABLE));
+            exchange.close();
+        }
+
         @Override
         protected String requestUniqueId(final HttpExchange exchange) {
             final String requestId = exchange.getRequestHeaders().getFirst(Constants.HeaderConstants.CLIENT_REQUEST_ID_HEADER);

+ 74 - 0
plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/TestUtils.java

@@ -0,0 +1,74 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.repositories.azure;
+
+import com.microsoft.azure.storage.Constants;
+import com.microsoft.azure.storage.StorageErrorCodeStrings;
+import com.sun.net.httpserver.Headers;
+import com.sun.net.httpserver.HttpExchange;
+import org.elasticsearch.common.SuppressForbidden;
+import org.elasticsearch.rest.RestStatus;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+
+final class TestUtils {
+
+    private TestUtils() {}
+
+    @SuppressForbidden(reason = "use HttpExchange and Headers")
+    static void sendError(final HttpExchange exchange, final RestStatus status) throws IOException {
+        final Headers headers = exchange.getResponseHeaders();
+        headers.add("Content-Type", "application/xml");
+
+        final String requestId = exchange.getRequestHeaders().getFirst(Constants.HeaderConstants.CLIENT_REQUEST_ID_HEADER);
+        if (requestId != null) {
+            headers.add(Constants.HeaderConstants.REQUEST_ID_HEADER, requestId);
+        }
+
+        final String errorCode = toAzureErrorCode(status);
+        if (errorCode != null) {
+            headers.add(Constants.HeaderConstants.ERROR_CODE, errorCode);
+        }
+
+        if (errorCode == null || "HEAD".equals(exchange.getRequestMethod())) {
+            exchange.sendResponseHeaders(status.getStatus(), -1L);
+        } else {
+            final byte[] response = ("<?xml version=\"1.0\" encoding=\"UTF-8\"?><Error><Code>" + errorCode + "</Code><Message>"
+                + status + "</Message></Error>").getBytes(StandardCharsets.UTF_8);
+            exchange.sendResponseHeaders(status.getStatus(), response.length);
+            exchange.getResponseBody().write(response);
+        }
+    }
+
+    // See https://docs.microsoft.com/en-us/rest/api/storageservices/common-rest-api-error-codes
+    private static String toAzureErrorCode(final RestStatus status) {
+        assert status.getStatus() >= 400;
+        switch (status) {
+            case BAD_REQUEST:
+                return StorageErrorCodeStrings.INVALID_METADATA;
+            case NOT_FOUND:
+                return StorageErrorCodeStrings.BLOB_NOT_FOUND;
+            case INTERNAL_SERVER_ERROR:
+                return StorageErrorCodeStrings.INTERNAL_ERROR;
+            default:
+                return null;
+        }
+    }
+}

+ 1 - 1
test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESMockAPIBasedRepositoryIntegTestCase.java

@@ -165,7 +165,7 @@ public abstract class ESMockAPIBasedRepositoryIntegTestCase extends ESBlobStoreR
             }
         }
 
-        private void handleAsError(final HttpExchange exchange) throws IOException {
+        protected void handleAsError(final HttpExchange exchange) throws IOException {
             Streams.readFully(exchange.getRequestBody());
             exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, -1);
             exchange.close();