import java.util.Locale;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
+import org.apache.catalina.connector.ClientAbortException;
import org.picocontainer.Startable;
import org.sonar.api.server.ServerSide;
import org.sonar.api.server.ws.LocalConnector;
verifyRequest(action, request);
action.handler().handle(request, response);
} catch (IllegalArgumentException e) {
- sendErrors(response, 400, singletonList(e.getMessage()));
+ sendErrors(request, response, e, 400, singletonList(e.getMessage()));
} catch (BadRequestException e) {
- sendErrors(response, 400, e.errors());
+ sendErrors(request, response, e, 400, e.errors());
} catch (ServerException e) {
- sendErrors(response, e.httpCode(), singletonList(e.getMessage()));
+ sendErrors(request, response, e, e.httpCode(), singletonList(e.getMessage()));
} catch (Exception e) {
- Response.Stream stream = response.stream();
- if (stream instanceof ServletResponse.ServletStream && ((ServletResponse.ServletStream) stream).response().isCommitted()) {
- // Request has been aborted by the client, nothing can been done as Tomcat has committed the response
- LOGGER.debug("Request {} has been aborted by client, error is '{}'", request, e.getMessage());
- return;
- }
- LOGGER.error("Fail to process request " + request, e);
- // Sending exception message into response is a vulnerability. Error must be
- // displayed only in logs.
- sendErrors(response, 500, singletonList("An error has occurred. Please contact your administrator"));
+ sendErrors(request, response, e, 500, singletonList("An error has occurred. Please contact your administrator"));
}
}
return controller == null ? null : controller.action(actionKey);
}
- private static void sendErrors(Response response, int status, List<String> errors) {
+ private static void sendErrors(Request request, Response response, Exception exception, int status, List<String> errors) {
+ if (isRequestAbortedByClient(exception)) {
+ // do not pollute logs. We can't do anything -> use DEBUG level
+ // see org.sonar.server.ws.ServletResponse#output()
+ LOGGER.debug(String.format("Request %s has been aborted by client", request), exception);
+ if (!isResponseCommitted(response)) {
+ // can be useful for access.log
+ response.stream().setStatus(299);
+ }
+ return;
+ }
+
+ if (status == 500) {
+ // Sending exception message into response is a vulnerability. Error must be
+ // displayed only in logs.
+ LOGGER.error("Fail to process request " + request, exception);
+ }
+
Response.Stream stream = response.stream();
+ if (isResponseCommitted(response)) {
+ // status can't be changed
+ LOGGER.debug(String.format("Request %s failed during response streaming", request), exception);
+ return;
+ }
+
+ // response is not committed, status and content can be changed to return the error
if (stream instanceof ServletResponse.ServletStream) {
- if (((ServletResponse.ServletStream) stream).response().isCommitted()) {
- // streaming of response. It's no more possible to clear and reformat the response
- return;
- }
((ServletResponse.ServletStream) stream).reset();
}
stream.setStatus(status);
stream.setMediaType(MediaTypes.JSON);
-
try (JsonWriter json = JsonWriter.of(new OutputStreamWriter(stream.output(), StandardCharsets.UTF_8))) {
json.beginObject();
writeErrors(json, errors);
}
}
+ private static boolean isRequestAbortedByClient(Exception exception) {
+ return Throwables.getCausalChain(exception).stream().anyMatch(t -> t instanceof ClientAbortException);
+ }
+
+ private static boolean isResponseCommitted(Response response) {
+ Response.Stream stream = response.stream();
+ // Request has been aborted by the client or the response was partially streamed, nothing can been done as Tomcat has committed the response
+ return stream instanceof ServletResponse.ServletStream && ((ServletResponse.ServletStream) stream).response().isCommitted();
+ }
+
public static void writeErrors(JsonWriter json, List<String> errorMessages) {
if (errorMessages.isEmpty()) {
return;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
+import org.mockito.Mockito;
+import org.sonar.api.server.ws.Request;
import org.sonar.api.server.ws.Response;
import org.sonar.api.server.ws.WebService;
import org.sonar.api.server.ws.internal.ValidatingRequest;
import org.sonar.server.exceptions.BadRequestException;
import org.sonarqube.ws.MediaTypes;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Matchers.anyInt;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
public class WebServiceEngineTest {
}
@Test
- public void execute_request() {
- ValidatingRequest request = new TestRequest().setMethod("GET").setPath("/api/system/health");
- DumbResponse response = new DumbResponse();
- underTest.execute(request, response);
+ public void ws_returns_successful_response() {
+ ValidatingRequest request = new TestRequest().setPath("/api/ping");
- assertThat(response.stream().outputAsString()).isEqualTo("good");
+ DumbResponse response = run(request, new PingWs());
+
+ assertThat(response.stream().outputAsString()).isEqualTo("pong");
+ assertThat(response.stream().status()).isEqualTo(200);
}
@Test
- public void execute_request_when_path_does_not_begin_with_slash() {
- ValidatingRequest request = new TestRequest().setMethod("GET").setPath("/api/system/health");
- DumbResponse response = new DumbResponse();
- underTest.execute(request, response);
+ public void accept_path_that_does_not_start_with_slash() {
+ ValidatingRequest request = new TestRequest().setPath("api/ping");
+
+ DumbResponse response = run(request, new PingWs());
- assertThat(response.stream().outputAsString()).isEqualTo("good");
+ assertThat(response.stream().outputAsString()).isEqualTo("pong");
+ assertThat(response.stream().status()).isEqualTo(200);
}
@Test
- public void execute_request_with_action_suffix() {
- ValidatingRequest request = new TestRequest().setMethod("GET").setPath("/api/system/health");
- DumbResponse response = new DumbResponse();
- underTest.execute(request, response);
+ public void request_path_can_contain_valid_media_type() {
+ ValidatingRequest request = new TestRequest().setPath("api/ping.json");
- assertThat(response.stream().outputAsString()).isEqualTo("good");
+ DumbResponse response = run(request, new PingWs());
+
+ assertThat(response.stream().outputAsString()).isEqualTo("pong");
+ assertThat(response.stream().status()).isEqualTo(200);
}
@Test
}
@Test
- public void does_not_fail_to_render_error_message_having_percent() {
+ public void return_error_message_containing_character_percent() {
ValidatingRequest request = new TestRequest().setMethod("GET").setPath("/api/system/error_message_having_percent");
DumbResponse response = new DumbResponse();
}
@Test
- public void should_handle_headers() {
+ public void send_response_headers() {
DumbResponse response = new DumbResponse();
String name = "Content-Disposition";
String value = "attachment; filename=sonarqube.zip";
}
@Test
- public void does_not_fail_when_request_is_aborted_and_response_is_committed() throws Exception {
- ValidatingRequest request = new TestRequest().setMethod("GET").setPath("/api/system/fail_with_client_abort_exception");
- Response response = mock(Response.class);
- ServletResponse.ServletStream servletStream = mock(ServletResponse.ServletStream.class);
+ public void support_aborted_request_when_response_is_already_committed() {
+ ValidatingRequest request = new TestRequest().setPath("/api/system/fail_with_client_abort_exception");
+ Response response = mockServletResponse(true);
+
+ underTest.execute(request, response);
+
+ // response is committed (status is already sent), so status can't be changed
+ verify(response.stream(), never()).setStatus(anyInt());
+
+ assertThat(logTester.logs(LoggerLevel.DEBUG)).contains("Request /api/system/fail_with_client_abort_exception has been aborted by client");
+ }
+
+ @Test
+ public void support_aborted_request_when_response_is_not_committed() {
+ ValidatingRequest request = new TestRequest().setPath("/api/system/fail_with_client_abort_exception");
+ Response response = mockServletResponse(false);
+
+ underTest.execute(request, response);
+
+ verify(response.stream()).setStatus(299);
+ assertThat(logTester.logs(LoggerLevel.DEBUG)).contains("Request /api/system/fail_with_client_abort_exception has been aborted by client");
+ }
+
+ @Test
+ public void internal_error_when_response_is_already_committed() {
+ ValidatingRequest request = new TestRequest().setPath("/api/system/fail");
+ Response response = mockServletResponse(true);
+
+ underTest.execute(request, response);
+
+ // response is committed (status is already sent), so status can't be changed
+ verify(response.stream(), never()).setStatus(anyInt());
+ assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Fail to process request /api/system/fail");
+ }
+
+ @Test
+ public void internal_error_when_response_is_not_committed() {
+ ValidatingRequest request = new TestRequest().setPath("/api/system/fail");
+ Response response = mockServletResponse(false);
+
+ underTest.execute(request, response);
+
+ verify(response.stream()).setStatus(500);
+ assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Fail to process request /api/system/fail");
+ }
+
+ private static DumbResponse run(Request request, WebService... webServices) {
+ WebServiceEngine engine = new WebServiceEngine(webServices);
+ engine.start();
+ try {
+ DumbResponse response = new DumbResponse();
+ engine.execute(request, response);
+ return response;
+ } finally {
+ engine.stop();
+ }
+ }
+
+ private static Response mockServletResponse(boolean committed) {
+ Response response = mock(Response.class, Mockito.RETURNS_DEEP_STUBS);
+ ServletResponse.ServletStream servletStream = mock(ServletResponse.ServletStream.class, Mockito.RETURNS_DEEP_STUBS);
when(response.stream()).thenReturn(servletStream);
- HttpServletResponse httpServletResponse = mock(HttpServletResponse.class);
- when(httpServletResponse.isCommitted()).thenReturn(true);
+ HttpServletResponse httpServletResponse = mock(HttpServletResponse.class, Mockito.RETURNS_DEEP_STUBS);
+ when(httpServletResponse.isCommitted()).thenReturn(committed);
when(servletStream.response()).thenReturn(httpServletResponse);
- underTest.execute(request, response);
+ return response;
+ }
- assertThat(logTester.logs(LoggerLevel.DEBUG)).isNotEmpty();
+ private static class PingWs implements WebService {
+ @Override
+ public void define(Context context) {
+ NewController newController = context.createController("api");
+ createNewDefaultAction(newController, "ping")
+ .setHandler((request, response) -> response.stream().output().write("pong".getBytes(UTF_8)));
+ newController.done();
+ }
}
static class SystemWs implements WebService {
String param = request.param("required-parameter");
});
+ // simulate a client abort
createNewDefaultAction(newController, "fail_with_client_abort_exception")
.setHandler((request, response) -> {
throw new IllegalStateException("fail!", new ClientAbortException());
newController.done();
}
- private NewAction createNewDefaultAction(NewController controller, String key) {
- return controller
- .createAction(key)
- .setDescription("Dummy Description")
- .setSince("5.3")
- .setResponseExample(getClass().getResource("web-service-engine-test.txt"));
- }
+ }
+
+ private static WebService.NewAction createNewDefaultAction(WebService.NewController controller, String key) {
+ return controller
+ .createAction(key)
+ .setDescription("Dummy Description")
+ .setSince("5.3")
+ .setResponseExample(WebServiceEngineTest.class.getResource("web-service-engine-test.txt"));
}
}
projects / sonarqube.git / commitdiff