diff --git a/gradle/changelog/notifications_behind_nginx.yaml b/gradle/changelog/notifications_behind_nginx.yaml new file mode 100644 index 0000000000..d60b7bd690 --- /dev/null +++ b/gradle/changelog/notifications_behind_nginx.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: SSE for notifications behind nginx reverse proxy ([#1650](https://github.com/scm-manager/scm-manager/pull/1650)) diff --git a/scm-core/src/main/java/sonia/scm/sse/SseResponse.java b/scm-core/src/main/java/sonia/scm/sse/SseResponse.java new file mode 100644 index 0000000000..74678b1abc --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/sse/SseResponse.java @@ -0,0 +1,41 @@ +/* + * 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.sse; + +import javax.ws.rs.NameBinding; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Marks an JAX-RS Endpoint as SseResponse. + * An JAX-RS Filter will add some http headers to the response + * to avoid common problems with SSE such as caching or buffering behind a reverse proxy. + * + * @since 2.19.0 + */ +@NameBinding +@Retention(RetentionPolicy.RUNTIME) +public @interface SseResponse { +} diff --git a/scm-test/src/main/java/sonia/scm/web/RestDispatcher.java b/scm-test/src/main/java/sonia/scm/web/RestDispatcher.java index 9d32264952..a57f4ef202 100644 --- a/scm-test/src/main/java/sonia/scm/web/RestDispatcher.java +++ b/scm-test/src/main/java/sonia/scm/web/RestDispatcher.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.web; import com.fasterxml.jackson.databind.ObjectMapper; @@ -31,6 +31,7 @@ import org.jboss.resteasy.mock.MockDispatcherFactory; import org.jboss.resteasy.spi.Dispatcher; import org.jboss.resteasy.spi.HttpRequest; import org.jboss.resteasy.spi.HttpResponse; +import org.jboss.resteasy.spi.ResteasyProviderFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.AlreadyExistsException; @@ -74,6 +75,10 @@ public class RestDispatcher { exceptionMapper.registerException(exceptionClass, status); } + public ResteasyProviderFactory getProviderFactory() { + return dispatcher.getProviderFactory(); + } + public void putDefaultContextObject(Class clazz, T object) { dispatcher.getDefaultContextObjects().put(clazz, object); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/SseHeaderResponseFilter.java b/scm-webapp/src/main/java/sonia/scm/api/rest/SseHeaderResponseFilter.java new file mode 100644 index 0000000000..373cdd6ffd --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/SseHeaderResponseFilter.java @@ -0,0 +1,59 @@ +/* + * 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.rest; + +import sonia.scm.sse.SseResponse; + +import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.container.ContainerResponseContext; +import javax.ws.rs.container.ContainerResponseFilter; +import javax.ws.rs.core.CacheControl; +import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.ext.Provider; +import java.io.IOException; + +@Provider +@SseResponse +public class SseHeaderResponseFilter implements ContainerResponseFilter { + + @Override + public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException { + MultivaluedMap headers = responseContext.getHeaders(); + cacheControl(headers); + avoidNginxBuffering(headers); + } + + private void avoidNginxBuffering(MultivaluedMap headers) { + headers.add("X-Accel-Buffering", "no"); + } + + private void cacheControl(MultivaluedMap headers) { + CacheControl cacheControl = new CacheControl(); + cacheControl.setNoCache(true); + + headers.remove("Cache-Control"); + headers.add("Cache-Control", cacheControl); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/CacheControlResponseFilter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/CacheControlResponseFilter.java index 0dc1f739bc..97695c60b9 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/CacheControlResponseFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/CacheControlResponseFilter.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; import org.slf4j.Logger; @@ -43,12 +43,20 @@ public class CacheControlResponseFilter implements ContainerResponseFilter { @Override public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) { - if (!isCacheable(responseContext)) { + if (shouldAppendCachingHeader(responseContext)) { LOG.trace("add no-cache header to response"); responseContext.getHeaders().add("Cache-Control", "no-cache"); } } + private boolean shouldAppendCachingHeader(ContainerResponseContext responseContext) { + return !hasAlreadyCacheControl(responseContext) && !isCacheable(responseContext); + } + + private boolean hasAlreadyCacheControl(ContainerResponseContext responseContext) { + return responseContext.getHeaders().containsKey("Cache-Control"); + } + private boolean isCacheable(ContainerResponseContext responseContext) { return hasLastModifiedDate(responseContext) || hasEntityTag(responseContext); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NotificationResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NotificationResource.java index 10fcd079d5..629c1603c9 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NotificationResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NotificationResource.java @@ -41,6 +41,7 @@ import sonia.scm.security.SessionId; import sonia.scm.sse.Channel; import sonia.scm.sse.ChannelRegistry; import sonia.scm.sse.Registration; +import sonia.scm.sse.SseResponse; import sonia.scm.web.VndMediaType; import javax.inject.Inject; @@ -175,6 +176,7 @@ public class NotificationResource { mediaType = VndMediaType.ERROR_TYPE, schema = @Schema(implementation = ErrorDto.class) )) + @SseResponse public void subscribe(@Context Sse sse, @Context SseEventSink eventSink, @QueryParam(SessionId.PARAMETER) SessionId sessionId) { Channel channel = channelRegistry.channel(NotificationChannelId.current()); channel.register(new Registration(sessionId, sse, eventSink)); diff --git a/scm-webapp/src/test/java/sonia/scm/api/rest/SseHeaderResponseFilterTest.java b/scm-webapp/src/test/java/sonia/scm/api/rest/SseHeaderResponseFilterTest.java new file mode 100644 index 0000000000..7f7625d191 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/rest/SseHeaderResponseFilterTest.java @@ -0,0 +1,131 @@ +/* + * 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.rest; + +import org.assertj.core.api.AbstractStringAssert; +import org.jboss.resteasy.mock.MockHttpRequest; +import org.jboss.resteasy.mock.MockHttpResponse; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import sonia.scm.api.v2.CacheControlResponseFilter; +import sonia.scm.sse.SseResponse; +import sonia.scm.web.RestDispatcher; + +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import java.net.URISyntaxException; +import java.util.List; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; + +class SseHeaderResponseFilterTest { + + private RestDispatcher restDispatcher; + + @BeforeEach + void setUp() { + restDispatcher = new RestDispatcher(); + restDispatcher.getProviderFactory().register(new SseHeaderResponseFilter()); + restDispatcher.addSingletonResource(new FakeSseResource()); + } + + @Test + void shouldAddResponseHeaders() throws URISyntaxException { + MockHttpResponse response = invoke("/fake/sse"); + + assertThat(response.getStatus()).isEqualTo(200); + assertContentType(response, MediaType.SERVER_SENT_EVENTS_TYPE); + assertStringHeader(response, "Cache-Control").isEqualTo("no-cache, no-transform"); + assertStringHeader(response, "X-Accel-Buffering").isEqualTo("no"); + } + + @Test + void shouldSkipNonSseResponses() throws URISyntaxException { + MockHttpResponse response = invoke("/fake/non"); + + assertThat(response.getStatus()).isEqualTo(200); + assertContentType(response, MediaType.TEXT_PLAIN_TYPE); + assertStringHeader(response, "Cache-Control").isNull(); + assertStringHeader(response, "X-Accel-Buffering").isNull(); + } + + @Test + void shouldAddCacheControlOnlyOnce() throws URISyntaxException { + restDispatcher.getProviderFactory().register(new CacheControlResponseFilter()); + MockHttpResponse response = invoke("/fake/sse"); + + List values = response.getOutputHeaders() + .get("Cache-Control") + .stream() + .map(Object::toString) + .collect(Collectors.toList()); + + assertThat(values).containsOnly("no-cache, no-transform"); + } + + private void assertContentType(MockHttpResponse response, MediaType expected) { + MediaType contentType = (MediaType) response.getOutputHeaders().getFirst("Content-Type"); + assertThat(contentType.isCompatible(expected)).isTrue(); + } + + private AbstractStringAssert assertStringHeader(MockHttpResponse response, String headerName) { + Object value = response.getOutputHeaders().getFirst(headerName); + if (value != null) { + return assertThat(value.toString()); + } + return assertThat((String) null); + } + + private MockHttpResponse invoke(String uri) throws URISyntaxException { + MockHttpRequest request = MockHttpRequest.get(uri); + MockHttpResponse response = new MockHttpResponse(); + restDispatcher.invoke(request, response); + return response; + } + + @Path("/fake") + public static class FakeSseResource { + + @GET + @SseResponse + @Path("sse") + @Produces(MediaType.SERVER_SENT_EVENTS) + public String fakeSse() { + return "sse"; + } + + @GET + @Path("non") + @Produces(MediaType.TEXT_PLAIN) + public String nonSse() { + return "non-sse"; + } + + } + +} diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/CacheControlResponseFilterTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/CacheControlResponseFilterTest.java index 4c5fdae934..5090ff7669 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/CacheControlResponseFilterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/CacheControlResponseFilterTest.java @@ -21,14 +21,14 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.container.ContainerResponseContext; @@ -36,10 +36,12 @@ import javax.ws.rs.core.EntityTag; import javax.ws.rs.core.MultivaluedMap; import java.util.Date; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; -@RunWith(MockitoJUnitRunner.class) -public class CacheControlResponseFilterTest { +@ExtendWith(MockitoExtension.class) +class CacheControlResponseFilterTest { @Mock private ContainerRequestContext requestContext; @@ -50,22 +52,22 @@ public class CacheControlResponseFilterTest { @Mock private MultivaluedMap headers; - private CacheControlResponseFilter filter = new CacheControlResponseFilter(); + private final CacheControlResponseFilter filter = new CacheControlResponseFilter(); - @Before - public void setUpMocks() { + @BeforeEach + void setUpMocks() { when(responseContext.getHeaders()).thenReturn(headers); } @Test - public void filterShouldAddCacheControlHeader() { + void shouldAddCacheControlHeader() { filter.filter(requestContext, responseContext); verify(headers).add("Cache-Control", "no-cache"); } @Test - public void filterShouldNotSetHeaderIfLastModifiedIsNotNull() { + void shouldNotSetHeaderIfLastModifiedIsNotNull() { when(responseContext.getLastModified()).thenReturn(new Date()); filter.filter(requestContext, responseContext); @@ -74,7 +76,7 @@ public class CacheControlResponseFilterTest { } @Test - public void filterShouldNotSetHeaderIfEtagIsNotNull() { + void shouldNotSetHeaderIfEtagIsNotNull() { when(responseContext.getEntityTag()).thenReturn(new EntityTag("42")); filter.filter(requestContext, responseContext); @@ -82,4 +84,13 @@ public class CacheControlResponseFilterTest { verify(headers, never()).add("Cache-Control", "no-cache"); } + @Test + void shouldNotOverrideExistingCacheControl() { + when(headers.containsKey("Cache-Control")).thenReturn(true); + + filter.filter(requestContext, responseContext); + + verify(headers, never()).add("Cache-Control", "no-cache"); + } + }