Skip to content

NIFI-7488 Listening Port property on HandleHttpRequest is not validated when Variable registry is used #4297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
adarmiento wants to merge 1 commit into from
Closed

NIFI-7488 Listening Port property on HandleHttpRequest is not validated when Variable registry is used #4297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion ...ndard-processors/src/main/java/org/apache/nifi/processors/standard/HandleHttpRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.apache.nifi.annotation.notification.PrimaryNodeState;
import org.apache.nifi.components.AllowableValue;
import org.apache.nifi.components.PropertyDescriptor;
import org.apache.nifi.components.ValidationContext;
import org.apache.nifi.components.ValidationResult;
import org.apache.nifi.expression.ExpressionLanguageScope;
import org.apache.nifi.flowfile.FlowFile;
import org.apache.nifi.http.HttpContextMap;
Expand Down Expand Up @@ -77,6 +79,7 @@
import java.security.Principal;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
Expand Down Expand Up @@ -142,6 +145,8 @@ public class HandleHttpRequest extends AbstractProcessor {
private static final String MIME_TYPE__MULTIPART_FORM_DATA = "multipart/form-data";

private static final Pattern URL_QUERY_PARAM_DELIMITER = Pattern.compile("&");
private static final Long MIN_PORT = 0L;
private static final Long MAX_PORT = 65535L;

// Allowable values for client auth
public static final AllowableValue CLIENT_NONE = new AllowableValue("No Authentication", "No Authentication",
Expand All @@ -156,7 +161,7 @@ public class HandleHttpRequest extends AbstractProcessor {
.name("Listening Port")
.description("The Port to listen on for incoming HTTP requests")
.required(true)
.addValidator(StandardValidators.createLongValidator(0L, 65535L, true))
.addValidator(StandardValidators.createLongValidator(MIN_PORT, MAX_PORT, true))
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.defaultValue("80")
.build();
Expand Down Expand Up @@ -321,6 +326,24 @@ public Set<Relationship> getRelationships() {
return Collections.singleton(REL_SUCCESS);
}

@Override
protected Collection<ValidationResult> customValidate(final ValidationContext validationContext) {
final List<ValidationResult> results = new ArrayList<>();

final Long port = validationContext.getProperty(PORT).evaluateAttributeExpressions().asLong();
Copy link
Copy Markdown
Contributor

@MikeThomsen MikeThomsen May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you doing this? Is the addValidator call not validating items from the variable registry?

Copy link
Copy Markdown
Contributor Author

@adarmiento adarmiento May 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a unit test using invalid values (passing from the registry), calling the .assertValid() was not marking the processor as invalid

Copy link
Copy Markdown
Contributor

@pvillard31 pvillard31 May 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validators, most of the time, won't do anything if expression language is used to configure the property. I guess we could improve the validators based on the EL scope of the property: if the scope is Variable Registry, we should be able to validate the property even if EL is used.

if (port < MIN_PORT || port > MAX_PORT) {
results.add(new ValidationResult.Builder()
.subject("Listening Port")
.input(String.valueOf(port))
.valid(false)
.explanation(String.format("<Listening Port> property shoult be included between "
+ "inclusive %d and %d", MIN_PORT, MAX_PORT))
.build());
}

return results;
}

@OnScheduled
public void clearInit(){
initialized.set(false);
Expand Down
38 changes: 37 additions & 1 deletion ...-processors/src/test/java/org/apache/nifi/processors/standard/ITestHandleHttpRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,50 @@ public void tearDown() throws Exception {
}
}

@Test(timeout = 30000)
public void testValidation() throws InitializationException {
CountDownLatch serverReady = new CountDownLatch(1);
CountDownLatch requestSent = new CountDownLatch(1);

final Long validPort = 1234L;
final Long negativePort = -1234L;
final Long tooHighPort = 165535L;

processor = createProcessor(serverReady, requestSent);
final TestRunner runner = TestRunners.newTestRunner(processor);

final MockHttpContextMap contextMap = new MockHttpContextMap();
runner.addControllerService("http-context-map", contextMap);
runner.enableControllerService(contextMap);
runner.setProperty(HandleHttpRequest.HTTP_CONTEXT_MAP, "http-context-map");

// This configuration is invalid because the selected port is lower than 0.
runner.setVariable("listening_port", String.valueOf(negativePort));
runner.setProperty(HandleHttpRequest.PORT, "${listening_port}");
runner.assertNotValid();

// This configuration is invalid because the selected port is higher than 65535.
runner.setVariable("listening_port", String.valueOf(tooHighPort));
runner.setProperty(HandleHttpRequest.PORT, "${listening_port}");
runner.assertNotValid();

// This configuration is valid.
runner.setVariable("listening_port", String.valueOf(validPort));
runner.setProperty(HandleHttpRequest.PORT, "${listening_port}");
runner.assertValid();
}

@Test(timeout = 30000)
public void testRequestAddedToService() throws InitializationException, IOException, InterruptedException {
CountDownLatch serverReady = new CountDownLatch(1);
CountDownLatch requestSent = new CountDownLatch(1);
final int validPort = 1234;

processor = createProcessor(serverReady, requestSent);

final TestRunner runner = TestRunners.newTestRunner(processor);
runner.setProperty(HandleHttpRequest.PORT, "0");
runner.setVariable("listening_port", String.valueOf(validPort));
runner.setProperty(HandleHttpRequest.PORT, "${listening_port}");

final MockHttpContextMap contextMap = new MockHttpContextMap();
runner.addControllerService("http-context-map", contextMap);
Expand All @@ -155,6 +190,7 @@ public void run() {
try {
serverReady.await();
final int port = ((HandleHttpRequest) runner.getProcessor()).getPort();
assertEquals(port, validPort);
final HttpURLConnection connection = (HttpURLConnection) new URL("http://localhost:"
+ port + "/my/path?query=true&value1=value1&value2=&value3&value4=apple=orange").openConnection();

Expand Down