From 42ae76ab7ca711fbb6adc2c4a9a01f37aa21fa2e Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 27 Sep 2019 09:30:00 +0200 Subject: [PATCH] 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: string-value string-value 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 --- .../azure/AzureBlobContainerRetriesTests.java | 16 ++-- .../azure/AzureBlobStoreRepositoryTests.java | 13 +++- .../repositories/azure/TestUtils.java | 74 +++++++++++++++++++ ...ESMockAPIBasedRepositoryIntegTestCase.java | 2 +- 4 files changed, 91 insertions(+), 14 deletions(-) create mode 100644 plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/TestUtils.java diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java index c5fe0c5e72f..0aa7a3b0922 100644 --- a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobContainerRetriesTests.java +++ b/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(); }); diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java index 2f12b1c61ff..28993bd475a 100644 --- a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureBlobStoreRepositoryTests.java +++ b/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); diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/TestUtils.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/TestUtils.java new file mode 100644 index 00000000000..cdb64ecbcf5 --- /dev/null +++ b/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 = ("" + errorCode + "" + + status + "").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; + } + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESMockAPIBasedRepositoryIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESMockAPIBasedRepositoryIntegTestCase.java index 21880d13683..e47bdeee3c2 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/blobstore/ESMockAPIBasedRepositoryIntegTestCase.java +++ b/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();