diff --git a/pom.xml b/pom.xml index 89259051db8d9..23737357f3acc 100644 --- a/pom.xml +++ b/pom.xml @@ -44,7 +44,7 @@ 3.3.9 4.7.1 - 0.209 + 0.215 ${dep.airlift.version} 0.38 0.6 diff --git a/presto-main/src/main/java/com/facebook/presto/memory/RemoteNodeMemory.java b/presto-main/src/main/java/com/facebook/presto/memory/RemoteNodeMemory.java index 82dc9725e4df8..7dc7de3f9c2f3 100644 --- a/presto-main/src/main/java/com/facebook/presto/memory/RemoteNodeMemory.java +++ b/presto-main/src/main/java/com/facebook/presto/memory/RemoteNodeMemory.java @@ -136,7 +136,7 @@ public void onSuccess(@Nullable BaseResponse result) memoryInfo.set(Optional.ofNullable(result.getValue())); } if (result.getStatusCode() != OK.code()) { - log.warn("Error fetching memory info from %s returned status %d: %s", memoryInfoUri, result.getStatusCode(), result.getStatusMessage()); + log.warn("Error fetching memory info from %s returned status %d", memoryInfoUri, result.getStatusCode()); return; } } diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/HttpRemoteNodeState.java b/presto-main/src/main/java/com/facebook/presto/metadata/HttpRemoteNodeState.java index cdbad264471d4..82eedad9f0a6e 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/HttpRemoteNodeState.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/HttpRemoteNodeState.java @@ -98,7 +98,7 @@ public void onSuccess(@Nullable JsonResponse result) nodeState.set(Optional.ofNullable(result.getValue())); } if (result.getStatusCode() != OK.code()) { - log.warn("Error fetching node state from %s returned status %d: %s", stateInfoUri, result.getStatusCode(), result.getStatusMessage()); + log.warn("Error fetching node state from %s returned status %d", stateInfoUri, result.getStatusCode()); return; } } diff --git a/presto-main/src/main/java/com/facebook/presto/operator/HttpRpcShuffleClient.java b/presto-main/src/main/java/com/facebook/presto/operator/HttpRpcShuffleClient.java index 676cb111c18e0..86eda2434c247 100644 --- a/presto-main/src/main/java/com/facebook/presto/operator/HttpRpcShuffleClient.java +++ b/presto-main/src/main/java/com/facebook/presto/operator/HttpRpcShuffleClient.java @@ -165,9 +165,8 @@ public PagesResponse handle(Request request, Response response) } throw new PageTransportErrorException( HostAddress.fromUri(request.getUri()), - format("Expected response code to be 200, but was %s %s:%n%s", + format("Expected response code to be 200, but was %s:%n%s", response.getStatusCode(), - response.getStatusMessage(), body.toString())); } diff --git a/presto-main/src/main/java/com/facebook/presto/server/SimpleHttpResponseHandler.java b/presto-main/src/main/java/com/facebook/presto/server/SimpleHttpResponseHandler.java index a88555eb36d64..0c1bd0971322c 100644 --- a/presto-main/src/main/java/com/facebook/presto/server/SimpleHttpResponseHandler.java +++ b/presto-main/src/main/java/com/facebook/presto/server/SimpleHttpResponseHandler.java @@ -82,18 +82,16 @@ else if (response.getStatusCode() == HttpStatus.SERVICE_UNAVAILABLE.code()) { private String createErrorMessage(BaseResponse response) { if (response instanceof JsonResponseWrapper) { - return format("Expected response code from %s to be %s, but was %s: %s%n%s", + return format("Expected response code from %s to be %s, but was %s: %n%s", uri, OK.code(), response.getStatusCode(), - response.getStatusMessage(), unwrapJsonResponse(response).getResponseBody()); } - return format("Expected response code from %s to be %s, but was %s: %s%n%s", + return format("Expected response code from %s to be %s, but was %s: %n%s", uri, OK.code(), response.getStatusCode(), - response.getStatusMessage(), new String(response.getResponseBytes())); } diff --git a/presto-main/src/main/java/com/facebook/presto/server/security/AuthenticationFilter.java b/presto-main/src/main/java/com/facebook/presto/server/security/AuthenticationFilter.java index 6e43f6739164b..96866a51594bf 100644 --- a/presto-main/src/main/java/com/facebook/presto/server/security/AuthenticationFilter.java +++ b/presto-main/src/main/java/com/facebook/presto/server/security/AuthenticationFilter.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.io.InputStream; +import java.io.PrintWriter; import java.security.Principal; import java.util.LinkedHashSet; import java.util.List; @@ -41,6 +42,7 @@ import static com.google.common.io.ByteStreams.copy; import static com.google.common.io.ByteStreams.nullOutputStream; import static com.google.common.net.HttpHeaders.WWW_AUTHENTICATE; +import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; import static java.util.Objects.requireNonNull; import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; @@ -109,7 +111,19 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo if (messages.isEmpty()) { messages.add("Unauthorized"); } - response.sendError(SC_UNAUTHORIZED, Joiner.on(" | ").join(messages)); + + // The error string is used by clients for exception messages and + // is presented to the end user, thus it should be a single line. + String error = Joiner.on(" | ").join(messages); + + // Clients should use the response body rather than the HTTP status + // message (which does not exist with HTTP/2), but the status message + // still needs to be sent for compatibility with existing clients. + response.setStatus(SC_UNAUTHORIZED, error); + response.setContentType(PLAIN_TEXT_UTF_8.toString()); + try (PrintWriter writer = response.getWriter()) { + writer.write(error); + } } private boolean doesRequestSupportAuthentication(HttpServletRequest request) diff --git a/presto-main/src/main/java/com/facebook/presto/server/smile/BaseResponse.java b/presto-main/src/main/java/com/facebook/presto/server/smile/BaseResponse.java index bb5dd0f23adda..893d454a0b246 100644 --- a/presto-main/src/main/java/com/facebook/presto/server/smile/BaseResponse.java +++ b/presto-main/src/main/java/com/facebook/presto/server/smile/BaseResponse.java @@ -26,8 +26,6 @@ public interface BaseResponse { int getStatusCode(); - String getStatusMessage(); - String getHeader(String name); List getHeaders(String name); diff --git a/presto-main/src/main/java/com/facebook/presto/server/smile/FullSmileResponseHandler.java b/presto-main/src/main/java/com/facebook/presto/server/smile/FullSmileResponseHandler.java index 20f5a39a5813f..bb5ed12e187bf 100644 --- a/presto-main/src/main/java/com/facebook/presto/server/smile/FullSmileResponseHandler.java +++ b/presto-main/src/main/java/com/facebook/presto/server/smile/FullSmileResponseHandler.java @@ -61,9 +61,9 @@ public SmileResponse handle(Request request, Response response) byte[] bytes = readResponseBytes(response); String contentType = response.getHeader(CONTENT_TYPE); if ((contentType == null) || !MediaType.parse(contentType).is(MEDIA_TYPE_SMILE)) { - return new SmileResponse<>(response.getStatusCode(), response.getStatusMessage(), response.getHeaders(), bytes); + return new SmileResponse<>(response.getStatusCode(), response.getHeaders(), bytes); } - return new SmileResponse<>(response.getStatusCode(), response.getStatusMessage(), response.getHeaders(), smileCodec, bytes); + return new SmileResponse<>(response.getStatusCode(), response.getHeaders(), smileCodec, bytes); } private static byte[] readResponseBytes(Response response) @@ -80,7 +80,6 @@ public static class SmileResponse implements BaseResponse { private final int statusCode; - private final String statusMessage; private final ListMultimap headers; private final boolean hasValue; private final byte[] smileBytes; @@ -88,10 +87,9 @@ public static class SmileResponse private final T value; private final IllegalArgumentException exception; - public SmileResponse(int statusCode, String statusMessage, ListMultimap headers, byte[] responseBytes) + public SmileResponse(int statusCode, ListMultimap headers, byte[] responseBytes) { this.statusCode = statusCode; - this.statusMessage = statusMessage; this.headers = ImmutableListMultimap.copyOf(headers); this.hasValue = false; @@ -102,10 +100,9 @@ public SmileResponse(int statusCode, String statusMessage, ListMultimap headers, SmileCodec smileCodec, byte[] smileBytes) + public SmileResponse(int statusCode, ListMultimap headers, SmileCodec smileCodec, byte[] smileBytes) { this.statusCode = statusCode; - this.statusMessage = statusMessage; this.headers = ImmutableListMultimap.copyOf(headers); this.smileBytes = requireNonNull(smileBytes, "smileBytes is null"); @@ -131,12 +128,6 @@ public int getStatusCode() return statusCode; } - @Override - public String getStatusMessage() - { - return statusMessage; - } - @Override public String getHeader(String name) { @@ -201,7 +192,6 @@ public String toString() { return toStringHelper(this) .add("statusCode", statusCode) - .add("statusMessage", statusMessage) .add("headers", headers) .add("hasValue", hasValue) .add("value", value) diff --git a/presto-main/src/main/java/com/facebook/presto/server/smile/JsonResponseWrapper.java b/presto-main/src/main/java/com/facebook/presto/server/smile/JsonResponseWrapper.java index 0f44aa33a3584..a4d0681bf1260 100644 --- a/presto-main/src/main/java/com/facebook/presto/server/smile/JsonResponseWrapper.java +++ b/presto-main/src/main/java/com/facebook/presto/server/smile/JsonResponseWrapper.java @@ -49,12 +49,6 @@ public int getStatusCode() return jsonResponse.getStatusCode(); } - @Override - public String getStatusMessage() - { - return jsonResponse.getStatusMessage(); - } - @Override public String getHeader(String name) { diff --git a/presto-main/src/main/java/com/facebook/presto/server/thrift/ThriftHttpResponseHandler.java b/presto-main/src/main/java/com/facebook/presto/server/thrift/ThriftHttpResponseHandler.java index 18e11836ebfec..04c902abbb330 100644 --- a/presto-main/src/main/java/com/facebook/presto/server/thrift/ThriftHttpResponseHandler.java +++ b/presto-main/src/main/java/com/facebook/presto/server/thrift/ThriftHttpResponseHandler.java @@ -83,11 +83,10 @@ else if (response.getStatusCode() == HttpStatus.SERVICE_UNAVAILABLE.code()) { private String createErrorMessage(ThriftResponse response) { - return format("Expected response code from %s to be %s, but was %s: %s%n%s", + return format("Expected response code from %s to be %s, but was %s: %n%s", uri, OK.code(), response.getStatusCode(), - response.getStatusMessage(), response.getValue()); } diff --git a/presto-main/src/test/java/com/facebook/presto/operator/TestPageBufferClient.java b/presto-main/src/test/java/com/facebook/presto/operator/TestPageBufferClient.java index 11893a5d98b57..8a4dc445561c8 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/TestPageBufferClient.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/TestPageBufferClient.java @@ -248,7 +248,7 @@ public void testInvalidResponses() assertEquals(callback.getFinishedBuffers(), 0); assertEquals(callback.getFailedBuffers(), 1); assertInstanceOf(callback.getFailure(), PageTransportErrorException.class); - assertContains(callback.getFailure().getCause().getMessage(), "Expected response code to be 200, but was 404 Not Found"); + assertContains(callback.getFailure().getCause().getMessage(), "Expected response code to be 200, but was 404"); assertStatus(client, location, "queued", 0, 1, 1, 1, "not scheduled"); // send invalid content type response and verify response was ignored diff --git a/presto-main/src/test/java/com/facebook/presto/server/TestQueryStateInfoResource.java b/presto-main/src/test/java/com/facebook/presto/server/TestQueryStateInfoResource.java index a2c433d654eb5..a7093dd971289 100644 --- a/presto-main/src/test/java/com/facebook/presto/server/TestQueryStateInfoResource.java +++ b/presto-main/src/test/java/com/facebook/presto/server/TestQueryStateInfoResource.java @@ -138,7 +138,7 @@ public void testGetQueryStateInfo() assertNotNull(info); } - @Test(expectedExceptions = {UnexpectedResponseException.class}, expectedExceptionsMessageRegExp = ".*404: Not Found") + @Test(expectedExceptions = {UnexpectedResponseException.class}, expectedExceptionsMessageRegExp = "Expected response code .*, but was 404") public void testGetQueryStateInfoNo() { client.execute( diff --git a/presto-product-tests/src/main/java/com/facebook/presto/tests/querystats/HttpQueryStatsClient.java b/presto-product-tests/src/main/java/com/facebook/presto/tests/querystats/HttpQueryStatsClient.java index e03c8fa42ae2f..9a65380bbb1e2 100644 --- a/presto-product-tests/src/main/java/com/facebook/presto/tests/querystats/HttpQueryStatsClient.java +++ b/presto-product-tests/src/main/java/com/facebook/presto/tests/querystats/HttpQueryStatsClient.java @@ -72,7 +72,7 @@ public Optional handle(Request request, Response response) return Optional.empty(); } else if (response.getStatusCode() != HttpStatus.OK.code()) { - throw new RuntimeException("unexpected error code " + response.getStatusCode() + "; reason=" + response.getStatusMessage()); + throw new RuntimeException("unexpected error code: " + response.getStatusCode()); } try { diff --git a/presto-router/src/main/java/com/facebook/presto/router/cluster/RemoteState.java b/presto-router/src/main/java/com/facebook/presto/router/cluster/RemoteState.java index ff27174aceab4..1d4a8eeb08dbf 100755 --- a/presto-router/src/main/java/com/facebook/presto/router/cluster/RemoteState.java +++ b/presto-router/src/main/java/com/facebook/presto/router/cluster/RemoteState.java @@ -91,7 +91,7 @@ public void onSuccess(@Nullable FullJsonResponseHandler.JsonResponse r handleResponse(result.getValue()); } if (result.getStatusCode() != OK.code()) { - log.warn("Error fetching node state from %s returned status %d: %s", remoteUri, result.getStatusCode(), result.getStatusMessage()); + log.warn("Error fetching node state from %s returned status %d", remoteUri, result.getStatusCode()); return; } } diff --git a/presto-router/src/main/java/com/facebook/presto/router/predictor/RemoteQuery.java b/presto-router/src/main/java/com/facebook/presto/router/predictor/RemoteQuery.java index e321fc083e5ea..ec31d614114ef 100644 --- a/presto-router/src/main/java/com/facebook/presto/router/predictor/RemoteQuery.java +++ b/presto-router/src/main/java/com/facebook/presto/router/predictor/RemoteQuery.java @@ -78,8 +78,8 @@ public synchronized void execute(String statement) if (result != null) { if (result.getStatusCode() != OK.code()) { log.error( - "Error fetching info from %s returned status %d: %s", - remoteUri, result.getStatusCode(), result.getStatusMessage()); + "Error fetching info from %s returned status %d", + remoteUri, result.getStatusCode()); } if (result.hasValue()) { handleResponse(result.getValue()); diff --git a/presto-spark-base/src/main/java/com/facebook/presto/spark/execution/http/PrestoSparkHttpTaskClient.java b/presto-spark-base/src/main/java/com/facebook/presto/spark/execution/http/PrestoSparkHttpTaskClient.java index 61a21d8743e2f..a013632ec06cd 100644 --- a/presto-spark-base/src/main/java/com/facebook/presto/spark/execution/http/PrestoSparkHttpTaskClient.java +++ b/presto-spark-base/src/main/java/com/facebook/presto/spark/execution/http/PrestoSparkHttpTaskClient.java @@ -364,7 +364,6 @@ public BaseResponse handle(Request request, Response response) { return new BytesResponse( response.getStatusCode(), - response.getStatusMessage(), response.getHeaders(), readResponseBytes(response)); } @@ -388,14 +387,12 @@ private static class BytesResponse implements BaseResponse { private final int statusCode; - private final String statusMessage; private final ListMultimap headers; private final byte[] bytes; - public BytesResponse(int statusCode, String statusMessage, ListMultimap headers, byte[] bytes) + public BytesResponse(int statusCode, ListMultimap headers, byte[] bytes) { this.statusCode = statusCode; - this.statusMessage = requireNonNull(statusMessage, "statusMessage is null"); this.headers = ImmutableListMultimap.copyOf(requireNonNull(headers, "headers is null")); this.bytes = bytes; } @@ -406,12 +403,6 @@ public int getStatusCode() return statusCode; } - @Override - public String getStatusMessage() - { - return statusMessage; - } - @Override public String getHeader(String name) { diff --git a/presto-spark-base/src/test/java/com/facebook/presto/spark/execution/http/TestPrestoSparkHttpClient.java b/presto-spark-base/src/test/java/com/facebook/presto/spark/execution/http/TestPrestoSparkHttpClient.java index 4233e237c1781..8af8d3e929234 100644 --- a/presto-spark-base/src/test/java/com/facebook/presto/spark/execution/http/TestPrestoSparkHttpClient.java +++ b/presto-spark-base/src/test/java/com/facebook/presto/spark/execution/http/TestPrestoSparkHttpClient.java @@ -283,7 +283,7 @@ public void testUpdateTaskUnexpectedResponse() TestingSession.testSessionBuilder().build(), createInitialEmptyOutputBuffers(PARTITIONED))) .isInstanceOf(PrestoException.class) - .hasMessageContaining("500: Internal Server Error"); + .hasMessageContaining("500"); } @Test @@ -802,7 +802,7 @@ public void testInfoFetcherUnexpectedResponse() } assertThatThrownBy(taskInfoFetcher::getTaskInfo) .isInstanceOf(PrestoException.class) - .hasMessageContaining("500: Internal Server Error"); + .hasMessageContaining("500"); } @Test @@ -1155,7 +1155,6 @@ public Response createServerInfoResponse() headers.put(HeaderName.of(CONTENT_TYPE), String.valueOf(MediaType.create("application", "json"))); return new TestingResponse( httpStatus.code(), - httpStatus.toString(), headers, new ByteArrayInputStream(serverInfoCodec.toBytes(serverInfo))); } @@ -1199,7 +1198,6 @@ protected Response createResultResponseHelper( headers.put(HeaderName.of(CONTENT_TYPE), PRESTO_PAGES_TYPE.toString()); return new TestingResponse( httpStatus.code(), - httpStatus.toString(), headers, slicedOutput.slice().getInput()); } @@ -1237,7 +1235,6 @@ public Response createTaskInfoResponse(HttpStatus httpStatus, String taskId) "dummy-node").withTaskStatus(createTaskStatusDone(location)); return new TestingResponse( httpStatus.code(), - httpStatus.toString(), headers, new ByteArrayInputStream(taskInfoCodec.toBytes(taskInfo))); } @@ -1290,25 +1287,21 @@ public static class TestingResponse implements Response { private final int statusCode; - private final String statusMessage; private final ListMultimap headers; private InputStream inputStream; private TestingResponse() { this.statusCode = HttpStatus.OK.code(); - this.statusMessage = HttpStatus.OK.toString(); this.headers = ArrayListMultimap.create(); } private TestingResponse( int statusCode, - String statusMessage, ListMultimap headers, InputStream inputStream) { this.statusCode = statusCode; - this.statusMessage = statusMessage; this.headers = headers; this.inputStream = inputStream; } @@ -1319,12 +1312,6 @@ public int getStatusCode() return statusCode; } - @Override - public String getStatusMessage() - { - return statusMessage; - } - @Override public ListMultimap getHeaders() { diff --git a/presto-tests/src/test/java/com/facebook/presto/resourcemanager/TestDistributedResourceGroupInfoResource.java b/presto-tests/src/test/java/com/facebook/presto/resourcemanager/TestDistributedResourceGroupInfoResource.java index 619cedb37b448..c74921c9cc129 100644 --- a/presto-tests/src/test/java/com/facebook/presto/resourcemanager/TestDistributedResourceGroupInfoResource.java +++ b/presto-tests/src/test/java/com/facebook/presto/resourcemanager/TestDistributedResourceGroupInfoResource.java @@ -102,7 +102,7 @@ public void testGetResourceGroupInfo() assertTrue(subGroupResourceIdSet.contains(new ResourceGroupId(resourceGroupInfo.getId(), "user-user2"))); } - @Test(expectedExceptions = UnexpectedResponseException.class, expectedExceptionsMessageRegExp = ".*404: Not Found") + @Test(expectedExceptions = UnexpectedResponseException.class, expectedExceptionsMessageRegExp = ".*404") public void testResourceGroup404() { getResponseEntity(client, coordinator1, "/v1/resourceGroupState/global1", JsonCodec.jsonCodec(ResourceGroupInfo.class)); diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/resolver/ClusterSizeFetcher.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/resolver/ClusterSizeFetcher.java index 5cfc946846bfe..504e394d0492b 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/resolver/ClusterSizeFetcher.java +++ b/presto-verifier/src/main/java/com/facebook/presto/verifier/resolver/ClusterSizeFetcher.java @@ -73,9 +73,8 @@ private int fetchClusterSize() StringResponse response = httpClient.execute(request, createStringResponseHandler()); checkState( response.getStatusCode() == OK.getStatusCode(), - "Invalid response: %s %s", - response.getStatusCode(), - response.getStatusMessage()); + "Invalid response: %s", + response.getStatusCode()); List> values; try {