diff --git a/src/main/java/uk/ac/ox/ctl/RestTemplateConfiguration.java b/src/main/java/uk/ac/ox/ctl/RestTemplateConfiguration.java index d53bb61..4804ca9 100644 --- a/src/main/java/uk/ac/ox/ctl/RestTemplateConfiguration.java +++ b/src/main/java/uk/ac/ox/ctl/RestTemplateConfiguration.java @@ -21,8 +21,8 @@ public class RestTemplateConfiguration { public RestTemplate restTemplate(RestTemplateBuilder restTemplateBuilder) { RestTemplate restTemplate = restTemplateBuilder - .setConnectTimeout(Duration.ofSeconds(5)) - .setReadTimeout(Duration.ofSeconds(5)) + .setConnectTimeout(Duration.ofSeconds(10)) + .setReadTimeout(Duration.ofSeconds(60)) .build(); restTemplate.setErrorHandler(new ResponseErrorHandler() { diff --git a/src/main/java/uk/ac/ox/ctl/canvasproxy/FilterConfig.java b/src/main/java/uk/ac/ox/ctl/canvasproxy/FilterConfig.java new file mode 100644 index 0000000..5bcc97e --- /dev/null +++ b/src/main/java/uk/ac/ox/ctl/canvasproxy/FilterConfig.java @@ -0,0 +1,20 @@ +package uk.ac.ox.ctl.canvasproxy; + +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class FilterConfig { + + @Bean + public FilterRegistrationBean viaFilter() { + FilterRegistrationBean registrationBean = new FilterRegistrationBean<>(); + + registrationBean.setFilter(new ViaFilter()); + registrationBean.addUrlPatterns("/api/*"); // All the proxy URLs + registrationBean.setOrder(1); + + return registrationBean; + } +} \ No newline at end of file diff --git a/src/main/java/uk/ac/ox/ctl/canvasproxy/ProxyController.java b/src/main/java/uk/ac/ox/ctl/canvasproxy/ProxyController.java index 0bed527..3f4ca1e 100644 --- a/src/main/java/uk/ac/ox/ctl/canvasproxy/ProxyController.java +++ b/src/main/java/uk/ac/ox/ctl/canvasproxy/ProxyController.java @@ -90,7 +90,7 @@ public ResponseEntity proxy(AbstractOAuth2TokenAuthenticationToken principal, } catch (ResourceAccessException e) { // When we get a timeout we should translate it to the correct HTTP message. if (e.getCause() instanceof SocketTimeoutException) { - throw new ResponseStatusException(HttpStatus.GATEWAY_TIMEOUT); + throw new ResponseStatusException(HttpStatus.GATEWAY_TIMEOUT, "Socket timeout accessing " + thirdPartyApi, e); } log.warn("Failed to load {} exception is: {}", thirdPartyApi, e.getMessage()); // TODO - Need to fix this so we don't log exception as this will be expected in production diff --git a/src/main/java/uk/ac/ox/ctl/canvasproxy/ViaFilter.java b/src/main/java/uk/ac/ox/ctl/canvasproxy/ViaFilter.java new file mode 100644 index 0000000..7313ed6 --- /dev/null +++ b/src/main/java/uk/ac/ox/ctl/canvasproxy/ViaFilter.java @@ -0,0 +1,31 @@ +package uk.ac.ox.ctl.canvasproxy; + +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import java.io.IOException; + +/** + * Adds on a Via header to indicate that the request passed through this proxy. + * This allows for easier debugging of requests. This is done in a filter as it's dealing with low-level concerns and + * doesn't fit well in the controller. + */ +public class ViaFilter implements Filter { + @Override + public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { + if (servletRequest instanceof HttpServletRequest request && servletResponse instanceof HttpServletResponse response) { + String existingVia = response.getHeader("Via"); + String newVia = request.getProtocol() + " " + request.getServerName() + " tool-support"; + if (existingVia != null && !existingVia.isEmpty()) { + newVia = existingVia + ", " + newVia; + } + response.setHeader("Via", newVia); + } + filterChain.doFilter(servletRequest, servletResponse); + } +} diff --git a/src/test/java/uk/ac/ox/ctl/canvasproxy/ViaFilterTest.java b/src/test/java/uk/ac/ox/ctl/canvasproxy/ViaFilterTest.java new file mode 100644 index 0000000..8b0d616 --- /dev/null +++ b/src/test/java/uk/ac/ox/ctl/canvasproxy/ViaFilterTest.java @@ -0,0 +1,65 @@ +package uk.ac.ox.ctl.canvasproxy; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; + +import static org.mockito.Mockito.*; + +class ViaFilterTest { + + private ViaFilter filter; + private HttpServletRequest request; + private HttpServletResponse response; + private FilterChain chain; + + @BeforeEach + void setUp() { + filter = new ViaFilter(); + request = mock(HttpServletRequest.class); + response = mock(HttpServletResponse.class); + chain = mock(FilterChain.class); + } + + @Test + void shouldAddViaHeaderWhenNoneExists() throws Exception { + when(request.getProtocol()).thenReturn("HTTP/1.1"); + when(request.getServerName()).thenReturn("localhost"); + when(response.getHeader("Via")).thenReturn(null); + + filter.doFilter(request, response, chain); + + verify(response).setHeader("Via", "HTTP/1.1 localhost tool-support"); + verify(chain).doFilter(request, response); + } + + @Test + void shouldAppendToExistingViaHeader() throws Exception { + when(request.getProtocol()).thenReturn("HTTP/2"); + when(request.getServerName()).thenReturn("myserver"); + when(response.getHeader("Via")).thenReturn("HTTP/1.1 proxy1"); + + filter.doFilter(request, response, chain); + + verify(response).setHeader("Via", "HTTP/1.1 proxy1, HTTP/2 myserver tool-support"); + verify(chain).doFilter(request, response); + } + + @Test + void shouldHandleNonHttpRequestsGracefully() throws IOException, jakarta.servlet.ServletException { + ServletRequest genericRequest = mock(ServletRequest.class); + ServletResponse genericResponse = mock(ServletResponse.class); + + filter.doFilter(genericRequest, genericResponse, chain); + + verify(chain).doFilter(genericRequest, genericResponse); + // no headers should be set + verifyNoInteractions(response); + } +} \ No newline at end of file