Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
<air.maven.version>3.3.9</air.maven.version>

<dep.antlr.version>4.7.1</dep.antlr.version>
<dep.airlift.version>0.209</dep.airlift.version>
<dep.airlift.version>0.215</dep.airlift.version>
<dep.packaging.version>${dep.airlift.version}</dep.packaging.version>
<dep.slice.version>0.38</dep.slice.version>
<dep.testing-mysql-server-5.version>0.6</dep.testing-mysql-server-5.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public void onSuccess(@Nullable BaseResponse<MemoryInfo> 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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void onSuccess(@Nullable JsonResponse<NodeState> 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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,16 @@ else if (response.getStatusCode() == HttpStatus.SERVICE_UNAVAILABLE.code()) {
private String createErrorMessage(BaseResponse<T> 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()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ public interface BaseResponse<T>
{
int getStatusCode();

String getStatusMessage();

String getHeader(String name);

List<String> getHeaders(String name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ public SmileResponse<T> 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)
Expand All @@ -80,18 +80,16 @@ public static class SmileResponse<T>
implements BaseResponse<T>
{
private final int statusCode;
private final String statusMessage;
private final ListMultimap<HeaderName, String> headers;
private final boolean hasValue;
private final byte[] smileBytes;
private final byte[] responseBytes;
private final T value;
private final IllegalArgumentException exception;

public SmileResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, byte[] responseBytes)
public SmileResponse(int statusCode, ListMultimap<HeaderName, String> headers, byte[] responseBytes)
{
this.statusCode = statusCode;
this.statusMessage = statusMessage;
this.headers = ImmutableListMultimap.copyOf(headers);

this.hasValue = false;
Expand All @@ -102,10 +100,9 @@ public SmileResponse(int statusCode, String statusMessage, ListMultimap<HeaderNa
}

@SuppressWarnings("ThrowableInstanceNeverThrown")
public SmileResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, SmileCodec<T> smileCodec, byte[] smileBytes)
public SmileResponse(int statusCode, ListMultimap<HeaderName, String> headers, SmileCodec<T> smileCodec, byte[] smileBytes)
{
this.statusCode = statusCode;
this.statusMessage = statusMessage;
this.headers = ImmutableListMultimap.copyOf(headers);

this.smileBytes = requireNonNull(smileBytes, "smileBytes is null");
Expand All @@ -131,12 +128,6 @@ public int getStatusCode()
return statusCode;
}

@Override
public String getStatusMessage()
{
return statusMessage;
}

@Override
public String getHeader(String name)
{
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ public int getStatusCode()
return jsonResponse.getStatusCode();
}

@Override
public String getStatusMessage()
{
return jsonResponse.getStatusMessage();
}

@Override
public String getHeader(String name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,10 @@ else if (response.getStatusCode() == HttpStatus.SERVICE_UNAVAILABLE.code()) {

private String createErrorMessage(ThriftResponse<T> 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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public Optional<QueryStats> 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public void onSuccess(@Nullable FullJsonResponseHandler.JsonResponse<JsonNode> 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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,6 @@ public BaseResponse<byte[]> handle(Request request, Response response)
{
return new BytesResponse(
response.getStatusCode(),
response.getStatusMessage(),
response.getHeaders(),
readResponseBytes(response));
}
Expand All @@ -388,14 +387,12 @@ private static class BytesResponse
implements BaseResponse<byte[]>
{
private final int statusCode;
private final String statusMessage;
private final ListMultimap<HeaderName, String> headers;
private final byte[] bytes;

public BytesResponse(int statusCode, String statusMessage, ListMultimap<HeaderName, String> headers, byte[] bytes)
public BytesResponse(int statusCode, ListMultimap<HeaderName, String> 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;
}
Expand All @@ -406,12 +403,6 @@ public int getStatusCode()
return statusCode;
}

@Override
public String getStatusMessage()
{
return statusMessage;
}

@Override
public String getHeader(String name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ public void testUpdateTaskUnexpectedResponse()
TestingSession.testSessionBuilder().build(),
createInitialEmptyOutputBuffers(PARTITIONED)))
.isInstanceOf(PrestoException.class)
.hasMessageContaining("500: Internal Server Error");
.hasMessageContaining("500");
}

@Test
Expand Down Expand Up @@ -802,7 +802,7 @@ public void testInfoFetcherUnexpectedResponse()
}
assertThatThrownBy(taskInfoFetcher::getTaskInfo)
.isInstanceOf(PrestoException.class)
.hasMessageContaining("500: Internal Server Error");
.hasMessageContaining("500");
}

@Test
Expand Down Expand Up @@ -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)));
}
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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)));
}
Expand Down Expand Up @@ -1290,25 +1287,21 @@ public static class TestingResponse
implements Response
{
private final int statusCode;
private final String statusMessage;
private final ListMultimap<HeaderName, String> 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<HeaderName, String> headers,
InputStream inputStream)
{
this.statusCode = statusCode;
this.statusMessage = statusMessage;
this.headers = headers;
this.inputStream = inputStream;
}
Expand All @@ -1319,12 +1312,6 @@ public int getStatusCode()
return statusCode;
}

@Override
public String getStatusMessage()
{
return statusMessage;
}

@Override
public ListMultimap<HeaderName, String> getHeaders()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Map<String, Object>> values;
try {
Expand Down