diff --git a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandler.java b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandler.java index 0714232cb..7a4f31e0d 100644 --- a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandler.java +++ b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandler.java @@ -481,19 +481,21 @@ public HttpEventResponse handle(HttpEventRequest requestToProxy, ErrorCauses.CONNECTION_TO_SOURCE.name()); builder.body("Error connecting to source API: " + e.getMessage()); log.log(Level.SEVERE, "Error connecting to source API: " + e.getMessage(), e); - return builder.build(); + return writeAsyncErrorResponseIfNeeded(builder.build(), processingContext); } catch (SocketTimeoutException e) { - return buildSourceApiTimeoutErrorResponse(builder, e); + return writeAsyncErrorResponseIfNeeded( + buildSourceApiTimeoutErrorResponse(builder, e), processingContext); } catch (IOException e) { if (isSocketTimeoutException(e)) { - return buildSourceApiTimeoutErrorResponse(builder, e); + return writeAsyncErrorResponseIfNeeded( + buildSourceApiTimeoutErrorResponse(builder, e), processingContext); } builder.statusCode(HttpStatus.SC_INTERNAL_SERVER_ERROR); builder.header(ProcessedDataMetadataFields.ERROR.getHttpHeader(), ErrorCauses.CONNECTION_TO_SOURCE.name()); builder.body("Error communicating with source API: " + e.getMessage()); log.log(Level.SEVERE, "Error communicating with source API", e); - return builder.build(); + return writeAsyncErrorResponseIfNeeded(builder.build(), processingContext); } @@ -512,7 +514,7 @@ && isSafeMethod(requestToSourceApi.getRequestMethod())) { ErrorCauses.API_ERROR.name()); builder.body("Async redirect Location is not HTTPS; refusing to follow"); log.log(Level.WARNING, "Async redirect to non-HTTPS Location refused: {0}", locationUrl); - return builder.build(); + return writeAsyncErrorResponseIfNeeded(builder.build(), processingContext); } log.info("Async request received " + sourceApiResponse.getStatusCode() + " redirect; fetching content from Location header"); @@ -533,7 +535,7 @@ && isSafeMethod(requestToSourceApi.getRequestMethod())) { ErrorCauses.CONNECTION_TO_SOURCE.name()); builder.body("Error fetching content from redirect location: " + e.getMessage()); log.log(Level.SEVERE, "Error fetching content from redirect location", e); - return builder.build(); + return writeAsyncErrorResponseIfNeeded(builder.build(), processingContext); } } @@ -595,11 +597,9 @@ && isSafeMethod(requestToSourceApi.getRequestMethod())) { ErrorCauses.API_ERROR.name()); proxyResponseContent = original.getContentAsString(); - // q: in async case, perhaps we should write the error to the async output, too, for - // clarity??? could do it with metadata indicating the error to the caller, so it - // doesn't wait forever??? - // if versioning is enabled in the bucket, then subsequent successful calls will - // overwrite the error response + if (processingContext.getAsync()) { + writeAsyncErrorContent(original, ErrorCauses.API_ERROR.name(), processingContext); + } } // only if not async, write content to body of response @@ -827,6 +827,41 @@ boolean isSafeMethod(String method) { return "GET".equalsIgnoreCase(method) || "HEAD".equalsIgnoreCase(method); } + private HttpEventResponse writeAsyncErrorResponseIfNeeded(HttpEventResponse response, + ProcessingContext processingContext) throws IOException { + if (!processingContext.getAsync()) { + return response; + } + + String errorCause = Optional.ofNullable(response.getHeaders()) + .map(headers -> headers.get(ProcessedDataMetadataFields.ERROR.getHttpHeader())) + .filter(StringUtils::isNotBlank) + .orElse(ErrorCauses.UNKNOWN.name()); + + ProcessedContent errorContent = ProcessedContent.builder() + .contentType(ContentType.TEXT_PLAIN.getMimeType()) + .contentCharset(StandardCharsets.UTF_8) + .content(StringUtils.defaultString(response.getBody()).getBytes(StandardCharsets.UTF_8)) + .build(); + writeAsyncErrorContent(errorContent, errorCause, processingContext); + return response; + } + + private void writeAsyncErrorContent(ProcessedContent content, String errorCause, + ProcessingContext processingContext) throws IOException { + Map metadata = new HashMap<>(content.getMetadata()); + metadata.put(ProcessedDataMetadataFields.ERROR.getMetadataKey(), errorCause); + metadata.put(ProcessedDataMetadataFields.PROXY_VERSION.getMetadataKey(), + ProxyConstants.JAVA_SOURCE_CODE_VERSION); + + ProcessedContent asyncError = content.toBuilder() + .metadata(metadata) + .content(Objects.requireNonNullElse(content.getContent(), new byte[0])) + .build(); + + asyncSanitizedDataOutput.get().writeSanitized(asyncError, processingContext); + } + @SneakyThrows String parseRequestedTarget(HttpEventRequest request) { String targetHost = apiModeConfig.getTargetHostOrError(); diff --git a/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandlerTest.java b/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandlerTest.java index 476fae358..3fa6c723c 100644 --- a/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandlerTest.java +++ b/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandlerTest.java @@ -2,15 +2,19 @@ import co.worklytics.psoxy.ConfigRulesModule; import co.worklytics.psoxy.ControlHeader; +import co.worklytics.psoxy.ErrorCauses; import co.worklytics.psoxy.Pseudonymizer; import co.worklytics.psoxy.PseudonymizerImplFactory; +import co.worklytics.psoxy.ProcessedDataMetadataFields; import co.worklytics.psoxy.PsoxyModule; import co.worklytics.psoxy.RESTApiSanitizer; import co.worklytics.psoxy.RESTApiSanitizerFactory; import co.worklytics.psoxy.gateway.ApiModeConfig; import co.worklytics.psoxy.gateway.HttpEventRequest; import co.worklytics.psoxy.gateway.HttpEventResponse; +import co.worklytics.psoxy.gateway.ProcessedContent; import co.worklytics.psoxy.gateway.ProxyConfigProperty; +import co.worklytics.psoxy.gateway.output.ApiSanitizedDataOutput; import co.worklytics.psoxy.impl.RESTApiSanitizerImpl; import co.worklytics.psoxy.rules.RESTRules; import co.worklytics.psoxy.rules.RulesUtils; @@ -694,6 +698,21 @@ public LowLevelHttpResponse execute() throws IOException { .normalizeHeader(org.apache.http.HttpHeaders.CONTENT_TYPE))); } + static class RecordingApiSanitizedDataOutput implements ApiSanitizedDataOutput { + + int writes; + ProcessedContent content; + ApiDataRequestHandler.ProcessingContext processingContext; + + @Override + public void writeSanitized(ProcessedContent content, + ApiDataRequestHandler.ProcessingContext processingContext) { + this.writes++; + this.content = content; + this.processingContext = processingContext; + } + } + private void setup(String source, String host) { ApiDataRequestHandlerTest.Container container = DaggerApiDataRequestHandlerTest_Container.create(); @@ -914,6 +933,125 @@ public LowLevelHttpResponse execute() throws IOException { assertEquals(redirectedContent, sanitizedBodyCaptor.getValue()); } + @Test + @SneakyThrows + void handleShouldWriteSourceApiErrorsToAsyncOutput() { + setup("gmail", "google.apis.com"); + + ApiDataRequestHandler spy = spy(handler); + + String errorContent = "{\"error\":\"source unavailable\"}"; + + HttpEventRequest request = MockModules.provideMock(HttpEventRequest.class); + when(request.getHeader(ControlHeader.PSEUDONYM_IMPLEMENTATION.getHttpHeader())) + .thenReturn(Optional.empty()); + when(request.getHttpMethod()).thenReturn("GET"); + when(request.getPath()).thenReturn("/admin/directory/v1/users"); + when(request.getQuery()).thenReturn(Optional.empty()); + + MockHttpTransport sourceTransport = new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildRequest(String method, String url) throws IOException { + return new MockLowLevelHttpRequest() { + @Override + public LowLevelHttpResponse execute() throws IOException { + MockLowLevelHttpResponse response = new MockLowLevelHttpResponse(); + response.setStatusCode(HttpStatus.SC_INTERNAL_SERVER_ERROR); + response.setContentType(Json.MEDIA_TYPE); + response.setContent(errorContent); + return response; + } + }; + } + }; + doReturn(sourceTransport.createRequestFactory()).when(spy).getRequestFactory(any()); + + RESTApiSanitizerImpl sanitizer = mock(RESTApiSanitizerImpl.class); + when(sanitizer.isAllowed(anyString(), any(), anyString(), any())).thenReturn(true); + when(sanitizer.getAllowedRequestHeaders(anyString(), any())).thenReturn(Optional.empty()); + spy.sanitizer = sanitizer; + + RecordingApiSanitizedDataOutput asyncOutput = new RecordingApiSanitizedDataOutput(); + spy.asyncSanitizedDataOutput = () -> asyncOutput; + ApiDataRequestHandler.ProcessingContext processingContext = + ApiDataRequestHandler.ProcessingContext.builder() + .async(true) + .requestId("r") + .asyncOutputLocation("gs://bucket/output.json") + .requestReceivedAt(clock.instant()) + .build(); + + HttpEventResponse response = spy.handle(request, processingContext); + + assertEquals(HttpStatus.SC_INTERNAL_SERVER_ERROR, response.getStatusCode()); + assertNull(response.getBody()); + + assertEquals(1, asyncOutput.writes); + assertSame(processingContext, asyncOutput.processingContext); + ProcessedContent asyncContent = asyncOutput.content; + assertEquals(errorContent, asyncContent.getContentAsString()); + assertEquals(ErrorCauses.API_ERROR.name(), asyncContent.getMetadata() + .get(ProcessedDataMetadataFields.ERROR.getMetadataKey())); + } + + @Test + @SneakyThrows + void handleShouldWriteAsyncRedirectFailuresToAsyncOutput() { + setup("gmail", "google.apis.com"); + + ApiDataRequestHandler spy = spy(handler); + + HttpEventRequest request = MockModules.provideMock(HttpEventRequest.class); + when(request.getHeader(ControlHeader.PSEUDONYM_IMPLEMENTATION.getHttpHeader())) + .thenReturn(Optional.empty()); + when(request.getHttpMethod()).thenReturn("GET"); + when(request.getPath()).thenReturn("/admin/directory/v1/users"); + when(request.getQuery()).thenReturn(Optional.empty()); + + MockHttpTransport redirectTransport = new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildRequest(String method, String url) throws IOException { + return new MockLowLevelHttpRequest() { + @Override + public LowLevelHttpResponse execute() throws IOException { + MockLowLevelHttpResponse response = new MockLowLevelHttpResponse(); + response.setStatusCode(307); + response.addHeader("location", "http://pre-signed.example.com/data.json"); + response.setContent(""); + return response; + } + }; + } + }; + doReturn(redirectTransport.createRequestFactory()).when(spy).getRequestFactory(any()); + + RESTApiSanitizerImpl sanitizer = mock(RESTApiSanitizerImpl.class); + when(sanitizer.isAllowed(anyString(), any(), anyString(), any())).thenReturn(true); + when(sanitizer.getAllowedRequestHeaders(anyString(), any())).thenReturn(Optional.empty()); + spy.sanitizer = sanitizer; + + RecordingApiSanitizedDataOutput asyncOutput = new RecordingApiSanitizedDataOutput(); + spy.asyncSanitizedDataOutput = () -> asyncOutput; + ApiDataRequestHandler.ProcessingContext processingContext = + ApiDataRequestHandler.ProcessingContext.builder() + .async(true) + .requestId("r") + .asyncOutputLocation("gs://bucket/output.json") + .requestReceivedAt(clock.instant()) + .build(); + + HttpEventResponse response = spy.handle(request, processingContext); + + assertEquals(HttpStatus.SC_BAD_GATEWAY, response.getStatusCode()); + + assertEquals(1, asyncOutput.writes); + assertSame(processingContext, asyncOutput.processingContext); + ProcessedContent asyncContent = asyncOutput.content; + assertTrue(asyncContent.getContentAsString().contains("Async redirect Location is not HTTPS")); + assertEquals(ErrorCauses.API_ERROR.name(), asyncContent.getMetadata() + .get(ProcessedDataMetadataFields.ERROR.getMetadataKey())); + } + @Test @SneakyThrows void handleShouldLetHttpClientFollowRedirectInSyncMode() { diff --git a/tools/release/lib/verify-bundles-lib.sh b/tools/release/lib/verify-bundles-lib.sh index c4874cb0b..bf70694f1 100644 --- a/tools/release/lib/verify-bundles-lib.sh +++ b/tools/release/lib/verify-bundles-lib.sh @@ -40,7 +40,6 @@ verify_bundles_version_from_tag() { fi printf '%s' "${BASH_REMATCH[1]}" } -} verify_bundles_artifact_names() { local version="$1"