[PLUGIN-1959] Add proxy support for ServiceNow plugins

[vikasrathee-cs]

Route all ServiceNow API calls (table reads, schema fetch, batch sink writes, OAuth token requests) through an optional HTTP proxy, modeled on the http-plugins proxy support.

• Add proxyUrl, proxyUsername, proxyPassword properties to ServiceNowConnectorConfig (nullable, macro-enabled) with validation

• Build a proxy-aware Apache HttpClient in RestAPIClient used by all GET/POST/DELETE calls

• Route the OAuth token call through the proxy via a new ProxyOltuHttpClient (Oltu HttpClient backed by Apache HttpClient); falls back to URLConnectionClient when no proxy is configured

• Add Proxy fields and a "Proxy authentication" filter to the connector, source, multi-source and sink widgets

• Document the new properties and add unit tests for proxy config

• ProxyOltuHttpClientTest: covers the OAuth-over-proxy client (POST token parsing, GET path, IOException wrapped as OAuthSystemException)

• RestAPIClientProxyTest: covers getHttpClientBuilder() proxy wiring (proxy host + credentials, proxy-only, no-proxy, invalid URL) by inspecting the real HttpClientBuilder's fields, since its setters are final.

Failed e2e are actully getting successful https://storage.googleapis.com/e2e-tests-cucumber-reports/servicenow-plugins/refs/pull/151/merge/cucumber-reports/advanced-reports/cucumber-html-reports/report-feature_8_566810399.html

And one e2e failure is due to slow response from service now instance and canCommit error is coming intermittently.

[gemini-code-assist]

Code Review

This pull request introduces proxy support for ServiceNow plugins, including batch sink, batch source, connector, and multi-source configurations. It adds proxy configuration fields (URL, username, password) to widgets, documentation, and config classes, and implements a custom ProxyOltuHttpClient to route OAuth token requests through the proxy. Feedback on the changes highlights several critical improvements: handling macro-enabled proxy properties during validation to prevent initialization errors, strengthening proxy URL validation to enforce scheme and port requirements, correctly instantiating HttpPut for PUT requests, restricting Content-Type headers to entity-enclosing requests, specifying UTF-8 encoding when parsing response entities, and using a case-insensitive map for HTTP headers.

[gemini-code-assist]

The validation of the proxyUrl using HttpHost.create(proxyUrl) is too lenient. It does not enforce that a protocol (scheme) and port are actually present, even though the error message and documentation state they are required. For example, proxy.example.com or http://proxy.example.com would pass validation but fail at runtime. Checking if the parsed host has a scheme and a valid port ensures robust validation.

    if (!containsMacro(ServiceNowConstants.PROPERTY_PROXY_URL) && !Util.isNullOrEmpty(proxyUrl)) {
      try {
        HttpHost host = HttpHost.create(proxyUrl);
        if (Util.isNullOrEmpty(host.getSchemeName()) || host.getPort() == -1) {
          throw new IllegalArgumentException("Proxy URL must contain a protocol and port.");
        }
      } catch (IllegalArgumentException e) {
        collector.addFailure("Proxy URL is not valid.",
                             "Ensure the Proxy URL contains a protocol, address and port. " +
                               "For example, http://proxy.example.com:8080.")
          .withConfigProperty(ServiceNowConstants.PROPERTY_PROXY_URL);
      }
    }

[gemini-code-assist]

If the requestMethod is PUT, the code incorrectly instantiates HttpPost instead of HttpPut. To support both POST and PUT correctly, they should be handled separately with their respective HTTP method classes.

    HttpRequestBase httpRequest;
    if (OAuth.HttpMethod.POST.equalsIgnoreCase(requestMethod)) {
      HttpPost httpPost = new HttpPost(request.getLocationUri());
      if (request.getBody() != null) {
        httpPost.setEntity(new StringEntity(request.getBody(), StandardCharsets.UTF_8));
      }
      httpRequest = httpPost;
    } else if (OAuth.HttpMethod.PUT.equalsIgnoreCase(requestMethod)) {
      org.apache.http.client.methods.HttpPut httpPut = new org.apache.http.client.methods.HttpPut(request.getLocationUri());
      if (request.getBody() != null) {
        httpPut.setEntity(new StringEntity(request.getBody(), StandardCharsets.UTF_8));
      }
      httpRequest = httpPut;
    } else {
      httpRequest = new HttpGet(request.getLocationUri());
    }

[gemini-code-assist]

The Content-Type header is unconditionally set to application/x-www-form-urlencoded if missing, even for GET requests. GET requests typically do not have a request body or a Content-Type header, and sending one can cause issues with some servers. It should only be set for entity-enclosing requests (like POST or PUT).

Suggested change

	if (httpRequest.getFirstHeader(OAuth.HeaderType.CONTENT_TYPE) == null) {
	httpRequest.setHeader(OAuth.HeaderType.CONTENT_TYPE, OAuth.ContentType.URL_ENCODED);
	}
	if (httpRequest instanceof org.apache.http.HttpEntityEnclosingRequest && httpRequest.getFirstHeader(OAuth.HeaderType.CONTENT_TYPE) == null) {
	httpRequest.setHeader(OAuth.HeaderType.CONTENT_TYPE, OAuth.ContentType.URL_ENCODED);
	}

[gemini-code-assist]

EntityUtils.toString(response.getEntity()) is called without specifying a default charset. If the response does not specify a charset in its Content-Type header, it may default to ISO-8859-1, which can corrupt non-ASCII characters. Specifying StandardCharsets.UTF_8 ensures robust parsing of the token response.

Suggested change

	String body = response.getEntity() != null ? EntityUtils.toString(response.getEntity()) : "";
	String body = response.getEntity() != null ? EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8) : "";

[gemini-code-assist]

HTTP header names are case-insensitive per RFC standards. Storing them in a standard HashMap can lead to issues if the caller looks up headers using a different casing (e.g., Content-Type vs content-type). Using a TreeMap with String.CASE_INSENSITIVE_ORDER ensures case-insensitive header lookups.

Suggested change

	private Map<String, List<String>> extractHeaders(CloseableHttpResponse response) {
	Map<String, List<String>> responseHeaders = new java.util.HashMap<>();
	for (Header header : response.getAllHeaders()) {
	List<String> values = responseHeaders.computeIfAbsent(header.getName(), k -> new ArrayList<>());
	values.add(header.getValue());
	}
	return Collections.unmodifiableMap(responseHeaders);
	}
	private Map<String, List<String>> extractHeaders(CloseableHttpResponse response) {
	Map<String, List<String>> responseHeaders = new java.util.TreeMap<>(String.CASE_INSENSITIVE_ORDER);
	for (Header header : response.getAllHeaders()) {
	List<String> values = responseHeaders.computeIfAbsent(header.getName(), k -> new ArrayList<>());
	values.add(header.getValue());
	}
	return Collections.unmodifiableMap(responseHeaders);
	}

[shruthi713]

As mentioned below by gemini-code-assist, the HttpHost.create() method in the Apache HttpComponents library is intentionally lenient and does accept URLs without an http:// or https:// prefix.so we might need to add an explicit check for those?

[vishwasvaidya-cloudsufi]

I verified this behavior. HttpHost.create() intentionally defaults to http when no scheme is provided. Based on the discussion, that's acceptable since users are expected to specify https:// explicitly when needed.

[shruthi713]

can we check below suggestion ? is it better to use conf.hasProxyConfigured() ? conf.getProxyUrl():null ?

[vishwasvaidya-cloudsufi]

• I tested the proxy macro (e.g., ${purl}) in CDAP, and it actually validated and executed successfully without this change.
• The crash the bot mentioned doesn't happen because CDAP automatically skips connection validation in the UI when it detects a macro.
• At runtime, the macro is fully resolved before the HTTP client is built, so no errors are thrown. Since the CDAP framework handles it , the change isn't necessary.