From 942d13c118be59cb360f7e334c87c0897da8e3fb Mon Sep 17 00:00:00 2001 From: dan-s1 Date: Wed, 28 Feb 2024 19:39:05 +0000 Subject: [PATCH] NIFI-12785 Corrected InvokeHTTP URL handling to avoid double encoding This closes #8458 Signed-off-by: David Handermann --- .../nifi/processors/standard/InvokeHTTP.java | 23 ++++++------- .../processors/standard/InvokeHTTPTest.java | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java index 7822d4d543..15a2740ed4 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java @@ -203,7 +203,8 @@ public class InvokeHTTP extends AbstractProcessor { public static final PropertyDescriptor HTTP_URL = new PropertyDescriptor.Builder() .name("HTTP URL") - .description("HTTP remote URL including a scheme of http or https, as well as a hostname or IP address with optional port and path elements.") + .description("HTTP remote URL including a scheme of http or https, as well as a hostname or IP address with optional port and path elements." + + " Any encoding of the URL must be done by the user.") .required(true) .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) .addValidator(StandardValidators.URL_VALIDATOR) @@ -852,19 +853,18 @@ public class InvokeHTTP extends AbstractProcessor { FlowFile responseFlowFile = null; try { final String urlProperty = trimToEmpty(context.getProperty(HTTP_URL).evaluateAttributeExpressions(requestFlowFile).getValue()); - final URL url = URLValidator.createURL(urlProperty); - Request httpRequest = configureRequest(context, session, requestFlowFile, url); + Request httpRequest = configureRequest(context, session, requestFlowFile, urlProperty); logRequest(logger, httpRequest); if (httpRequest.body() != null) { - session.getProvenanceReporter().send(requestFlowFile, url.toExternalForm(), true); + session.getProvenanceReporter().send(requestFlowFile, urlProperty, true); } final long startNanos = System.nanoTime(); try (Response responseHttp = okHttpClient.newCall(httpRequest).execute()) { - logResponse(logger, url, responseHttp); + logResponse(logger, urlProperty, responseHttp); // store the status code and message int statusCode = responseHttp.code(); @@ -874,7 +874,7 @@ public class InvokeHTTP extends AbstractProcessor { Map statusAttributes = new HashMap<>(); statusAttributes.put(STATUS_CODE, String.valueOf(statusCode)); statusAttributes.put(STATUS_MESSAGE, statusMessage); - statusAttributes.put(REQUEST_URL, url.toExternalForm()); + statusAttributes.put(REQUEST_URL, urlProperty); statusAttributes.put(REQUEST_DURATION, Long.toString(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos))); statusAttributes.put(RESPONSE_URL, responseHttp.request().url().toString()); statusAttributes.put(TRANSACTION_ID, txId.toString()); @@ -927,6 +927,7 @@ public class InvokeHTTP extends AbstractProcessor { // update FlowFile's filename attribute with an extracted value from the remote URL if (FlowFileNamingStrategy.URL_PATH.equals(getFlowFileNamingStrategy(context)) && HttpMethod.GET.name().equals(httpRequest.method())) { + final URL url = URLValidator.createURL(urlProperty); String fileName = getFileNameFromUrl(url); if (fileName != null) { responseFlowFile = session.putAttribute(responseFlowFile, CoreAttributes.FILENAME.key(), fileName); @@ -950,9 +951,9 @@ public class InvokeHTTP extends AbstractProcessor { // emit provenance event final long millis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos); if (requestFlowFile != null) { - session.getProvenanceReporter().fetch(responseFlowFile, url.toExternalForm(), millis); + session.getProvenanceReporter().fetch(responseFlowFile, urlProperty, millis); } else { - session.getProvenanceReporter().receive(responseFlowFile, url.toExternalForm(), millis); + session.getProvenanceReporter().receive(responseFlowFile, urlProperty, millis); } } } @@ -1012,7 +1013,7 @@ public class InvokeHTTP extends AbstractProcessor { } } - private Request configureRequest(final ProcessContext context, final ProcessSession session, final FlowFile requestFlowFile, URL url) { + private Request configureRequest(final ProcessContext context, final ProcessSession session, final FlowFile requestFlowFile, String url) { final Request.Builder requestBuilder = new Request.Builder(); requestBuilder.url(url); @@ -1231,10 +1232,10 @@ public class InvokeHTTP extends AbstractProcessor { } } - private void logResponse(ComponentLog logger, URL url, Response response) { + private void logResponse(ComponentLog logger, String url, Response response) { if (logger.isDebugEnabled()) { logger.debug("\nResponse from remote service:\n\t{}\n{}", - url.toExternalForm(), getLogString(response.headers().toMultimap())); + url, getLogString(response.headers().toMultimap())); } } diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/InvokeHTTPTest.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/InvokeHTTPTest.java index 532b853c14..6f69ce257f 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/InvokeHTTPTest.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/InvokeHTTPTest.java @@ -25,11 +25,14 @@ import org.apache.commons.lang3.StringUtils; import org.apache.nifi.flowfile.attributes.CoreAttributes; import org.apache.nifi.oauth2.OAuth2AccessTokenProvider; import org.apache.nifi.processor.Relationship; +import org.apache.nifi.processor.util.URLValidator; import org.apache.nifi.processors.standard.http.ContentEncodingStrategy; import org.apache.nifi.processors.standard.http.FlowFileNamingStrategy; import org.apache.nifi.processors.standard.http.CookieStrategy; import org.apache.nifi.processors.standard.http.HttpHeader; import org.apache.nifi.processors.standard.http.HttpMethod; +import org.apache.nifi.provenance.ProvenanceEventRecord; +import org.apache.nifi.provenance.ProvenanceEventType; import org.apache.nifi.proxy.ProxyConfiguration; import org.apache.nifi.proxy.ProxyConfigurationService; import org.apache.nifi.reporting.InitializationException; @@ -79,6 +82,7 @@ import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED; import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -548,6 +552,35 @@ public class InvokeHTTPTest { assertEquals(userAgent, userAgentHeader); } + @Test + void testRunGetHttp200SuccessWithEncodableUrl() throws Exception { + final String partialEncodableUrl = "/gitlab/ftp%2Fstage%2F15m%2FsomeFile.yaml/raw?ref=main"; + final String nonEncodedUrl = mockWebServer.url(partialEncodableUrl).newBuilder().host(LOCALHOST).build().toString(); + final String encodedUrl = URLValidator.createURL(nonEncodedUrl).toExternalForm(); + + runner.setProperty(InvokeHTTP.HTTP_URL, nonEncodedUrl); + mockWebServer.enqueue(new MockResponse().setResponseCode(HTTP_OK)); + runner.enqueue(FLOW_FILE_CONTENT); + runner.run(); + + assertResponseSuccessRelationships(); + assertRelationshipStatusCodeEquals(InvokeHTTP.RESPONSE, HTTP_OK); + + MockFlowFile flowFile = getResponseFlowFile(); + final String actualUrl = flowFile.getAttribute(InvokeHTTP.REQUEST_URL); + assertNotEquals(encodedUrl, actualUrl); + assertTrue(actualUrl.endsWith(partialEncodableUrl)); + + final ProvenanceEventRecord event = runner.getProvenanceEvents().stream() + .filter(record -> record.getEventType() == ProvenanceEventType.FETCH) + .findFirst() + .orElse(null); + assertNotNull(event); + final String transitUri = event.getTransitUri(); + assertNotEquals(encodedUrl, transitUri); + assertTrue(transitUri.endsWith(partialEncodableUrl)); + } + @Test public void testRunGetHttp302NoRetryResponseRedirectsDefaultEnabled() { mockWebServer.enqueue(new MockResponse().setResponseCode(HTTP_MOVED_TEMP).setHeader(LOCATION_HEADER, getMockWebServerUrl()));