From 4cbe6b9873884a476aedc63bc22eb0a97806a43f Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 25 Mar 2021 13:09:40 +0100 Subject: [PATCH] Improve error messages for invalid media types (#1607) Show separate error messages for invalid partial media types and application/json if it unsupported for the url. --- .../improve_invalid_media_type_error.yaml | 2 + .../scm/api/v2/InvalidAcceptHeaderFilter.java | 114 +++++++++ .../scm/api/v2/ResponseFilterPriorities.java | 4 +- .../api/v2/InvalidAcceptHeaderFilterTest.java | 221 ++++++++++++++++++ 4 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 gradle/changelog/improve_invalid_media_type_error.yaml create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/InvalidAcceptHeaderFilter.java create mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/InvalidAcceptHeaderFilterTest.java diff --git a/gradle/changelog/improve_invalid_media_type_error.yaml b/gradle/changelog/improve_invalid_media_type_error.yaml new file mode 100644 index 0000000000..044c43a139 --- /dev/null +++ b/gradle/changelog/improve_invalid_media_type_error.yaml @@ -0,0 +1,2 @@ +- type: changed + description: Improve error messages for invalid media types ([#1607](https://github.com/scm-manager/scm-manager/pull/1607)) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/InvalidAcceptHeaderFilter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/InvalidAcceptHeaderFilter.java new file mode 100644 index 0000000000..7112ea1d4e --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/InvalidAcceptHeaderFilter.java @@ -0,0 +1,114 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.api.v2; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; +import sonia.scm.TransactionId; +import sonia.scm.api.v2.resources.ErrorDto; + +import javax.annotation.Priority; +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.container.ContainerResponseContext; +import javax.ws.rs.container.ContainerResponseFilter; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.ext.Provider; +import java.util.List; + +@Provider +@Priority(ResponseFilterPriorities.INVALID_ACCEPT_HEADER) +public class InvalidAcceptHeaderFilter implements ContainerResponseFilter { + + @VisibleForTesting + static final String CODE_PARTIAL_WILDCARD = "ChSSf2AFs1"; + + private static final String MESSAGE_PARTIAL_WILDCARD = "Partial wildcards for media types are not supported. " + + "Please use a valid content type or a non partial wildcard such as application/*."; + + @VisibleForTesting + static final String CODE_APPLICATION_JSON = "7bSSf4F381"; + private static final String MESSAGE_APPLICATION_JSON = "Media type application/json is not supported on this url. " + + "Please use a valid vnd media type."; + + @Override + public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) { + if (isNoMatchForAcceptHeader(responseContext)) { + List acceptableMediaTypes = requestContext.getAcceptableMediaTypes(); + if (containsPartialWildcard(acceptableMediaTypes)) { + responseContext.setEntity(createPartialWildcardError()); + } else if (containsApplicationJson(acceptableMediaTypes)) { + responseContext.setEntity(createApplicationJsonError()); + } + } + } + + private boolean containsApplicationJson(List acceptableMediaTypes) { + return acceptableMediaTypes.stream().anyMatch(this::isApplicationJson); + } + + private ErrorDto createPartialWildcardError() { + ErrorDto errorDto = new ErrorDto(); + errorDto.setErrorCode(CODE_PARTIAL_WILDCARD); + errorDto.setMessage(MESSAGE_PARTIAL_WILDCARD); + TransactionId.get().ifPresent(errorDto::setTransactionId); + return errorDto; + } + + private ErrorDto createApplicationJsonError() { + ErrorDto errorDto = new ErrorDto(); + errorDto.setErrorCode(CODE_APPLICATION_JSON); + errorDto.setMessage(MESSAGE_APPLICATION_JSON); + TransactionId.get().ifPresent(errorDto::setTransactionId); + return errorDto; + } + + private boolean containsPartialWildcard(List acceptableMediaTypes) { + return acceptableMediaTypes.stream().anyMatch(this::isPartialWildcard); + } + + private boolean isPartialWildcard(MediaType mediaType) { + if (mediaType.getSubtype() != null) { + return mediaType.getSubtype().contains("*+json"); + } + return true; + } + + private boolean isApplicationJson(MediaType mediaType) { + return "application".equals(mediaType.getType()) && "json".equals(mediaType.getSubtype()); + } + + private boolean isNoMatchForAcceptHeader(ContainerResponseContext responseContext) { + if (responseContext.getStatus() == HttpServletResponse.SC_NOT_ACCEPTABLE) { + Object entity = responseContext.getEntity(); + if (entity instanceof ErrorDto) { + String message = ((ErrorDto) entity).getMessage(); + return Strings.nullToEmpty(message).startsWith("RESTEASY003635"); + } + } + return false; + } + +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/ResponseFilterPriorities.java b/scm-webapp/src/main/java/sonia/scm/api/v2/ResponseFilterPriorities.java index 791b0fdcc3..4002de0176 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/ResponseFilterPriorities.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/ResponseFilterPriorities.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2; @@ -40,6 +40,8 @@ final class ResponseFilterPriorities { static final int JSON_MARSHALLING = Priorities.USER + 1000; static final int FIELD_FILTER = Priorities.USER; + static final int INVALID_ACCEPT_HEADER = JSON_MARSHALLING + 1000; + private ResponseFilterPriorities() { } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/InvalidAcceptHeaderFilterTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/InvalidAcceptHeaderFilterTest.java new file mode 100644 index 0000000000..e7c060b700 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/InvalidAcceptHeaderFilterTest.java @@ -0,0 +1,221 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.api.v2; + +import org.jboss.resteasy.mock.MockDispatcherFactory; +import org.jboss.resteasy.mock.MockHttpRequest; +import org.jboss.resteasy.mock.MockHttpResponse; +import org.jboss.resteasy.spi.Dispatcher; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.api.WebApplicationExceptionMapper; +import sonia.scm.api.v2.resources.ErrorDto; +import sonia.scm.web.RestDispatcher; +import sonia.scm.web.VndMediaType; + +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.container.ContainerResponseContext; +import javax.ws.rs.core.MediaType; + +import java.io.UnsupportedEncodingException; +import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class InvalidAcceptHeaderFilterTest { + + @Mock + private ContainerRequestContext requestContext; + + @Mock + private ContainerResponseContext responseContext; + + @Captor + private ArgumentCaptor entityCaptor; + + private final InvalidAcceptHeaderFilter filter = new InvalidAcceptHeaderFilter(); + + @Test + void shouldIgnoreWithOtherStatusCode() { + when(responseContext.getStatus()).thenReturn(HttpServletResponse.SC_NOT_FOUND); + filter.filter(requestContext, responseContext); + verify(responseContext, never()).setEntity(any()); + } + + @Test + void shouldIgnoreWithoutErrorDto() { + when(responseContext.getStatus()).thenReturn(HttpServletResponse.SC_NOT_ACCEPTABLE); + when(responseContext.getEntity()).thenReturn("Hello"); + filter.filter(requestContext, responseContext); + verify(responseContext, never()).setEntity(any()); + } + + @Test + void shouldIgnoreWithoutResteasyError() { + when(responseContext.getStatus()).thenReturn(HttpServletResponse.SC_NOT_ACCEPTABLE); + ErrorDto dto = new ErrorDto(); + dto.setMessage("Other"); + when(responseContext.getEntity()).thenReturn(dto); + filter.filter(requestContext, responseContext); + verify(responseContext, never()).setEntity(any()); + } + + @Nested + class NoMatchForAcceptHeader { + + @BeforeEach + void setStatusCode() { + when(responseContext.getStatus()).thenReturn(HttpServletResponse.SC_NOT_ACCEPTABLE); + + ErrorDto error = new ErrorDto(); + error.setMessage("RESTEASY003635: No match for accept header"); + + when(responseContext.getEntity()).thenReturn(error); + } + + @Test + void shouldNotModifyResponse() { + mediaTypes(); + + filter.filter(requestContext, responseContext); + + verify(responseContext, never()).setEntity(any()); + } + + @Test + void shouldNotModifyResponseWithFullQualifiedAccept() { + mediaTypes(VndMediaType.ANNOTATE); + + filter.filter(requestContext, responseContext); + + verify(responseContext, never()).setEntity(any()); + } + + @Test + void shouldNotModifyResponseWithMultipleFullQualifiedAccept() { + mediaTypes(VndMediaType.ANNOTATE, VndMediaType.SOURCE, VndMediaType.BRANCH); + + filter.filter(requestContext, responseContext); + + verify(responseContext, never()).setEntity(any()); + } + + @Test + void shouldReturnErrorForPartialWildcard() { + mediaTypes("application/*+json"); + + filter.filter(requestContext, responseContext); + + assertError(InvalidAcceptHeaderFilter.CODE_PARTIAL_WILDCARD); + } + + @Test + void shouldReturnApplicationJsonError() { + mediaTypes("application/json"); + + filter.filter(requestContext, responseContext); + + assertError(InvalidAcceptHeaderFilter.CODE_APPLICATION_JSON); + } + + } + + @Nested + class Integration { + + private Dispatcher dispatcher; + + @BeforeEach + void prepare() { + dispatcher = MockDispatcherFactory.createDispatcher(); + dispatcher.getProviderFactory().registerProvider(InvalidAcceptHeaderFilter.class); + dispatcher.getProviderFactory().registerProvider(WebApplicationExceptionMapper.class); + dispatcher.getRegistry().addSingletonResource(new SampleResource()); + } + + @Test + void shouldReturnApplicationJsonError() throws URISyntaxException, UnsupportedEncodingException { + MockHttpRequest request = MockHttpRequest.get("/").accept("application/json"); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertThat(response.getContentAsString()).contains(InvalidAcceptHeaderFilter.CODE_APPLICATION_JSON); + } + + @Test + void shouldReturnErrorForPartialWildcard() throws URISyntaxException, UnsupportedEncodingException { + MockHttpRequest request = MockHttpRequest.get("/").accept("application/*+json"); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertThat(response.getContentAsString()).contains(InvalidAcceptHeaderFilter.CODE_PARTIAL_WILDCARD); + } + + } + + @Path("/") + class SampleResource { + + @GET + @Produces("text/plain") + public String hello() { + return "hello"; + } + + } + + private void assertError(String code) { + verify(responseContext).setEntity(entityCaptor.capture()); + Object value = entityCaptor.getValue(); + assertThat(value).isInstanceOfSatisfying(ErrorDto.class, error -> { + assertThat(error.getErrorCode()).isEqualTo(code); + }); + } + + private void mediaTypes(String... types) { + List mediaTypes = Arrays.stream(types) + .map(MediaType::valueOf).collect(Collectors.toList()); + + when(requestContext.getAcceptableMediaTypes()).thenReturn(mediaTypes); + } +}