Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 36 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
<testSourceLocation>${project.basedir}/src/test/java/</testSourceLocation>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<jetty.version>12.1.10</jetty.version>
<powermock.version>2.0.2</powermock.version>
<failsafe.version>3.3.2</failsafe.version>
</properties>
Expand All @@ -99,7 +100,17 @@
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-http</artifactId>
<version>9.4.52.v20230823</version>
<version>${jetty.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-client</artifactId>
<version>${jetty.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-util</artifactId>
<version>${jetty.version}</version>
</dependency>
<dependency>
<groupId>io.cdap.cdap</groupId>
Expand Down Expand Up @@ -270,6 +281,22 @@
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-http</artifactId>
</exclusion>
<exclusion>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Excluding Jetty 9.4.x transitive dependencies from cometd-java-client version 4.0.0 and forcing it to run with Jetty 12 will cause critical runtime failures (such as NoClassDefFoundError or NoSuchMethodError). CometD 4.x is compiled against Jetty 9.4.x APIs (e.g., it references org.eclipse.jetty.client.api.Request which has been refactored/removed in Jetty 12). To support Jetty 12, you must upgrade cometd-java-client to a compatible version (such as CometD 7.x or 8.x depending on the Jakarta EE namespace used by the project).

<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-client</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-util</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-util-ajax</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-io</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
Expand Down Expand Up @@ -377,6 +404,14 @@
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty</groupId>
<artifactId>*</artifactId>
</exclusion>
<exclusion>
<groupId>org.eclipse.jetty.orbit</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
<scope>test</scope>
</dependency>
Expand Down
17 changes: 6 additions & 11 deletions src/main/java/io/cdap/plugin/salesforce/SalesforceQueryUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@
import io.cdap.plugin.salesforce.authenticator.AuthenticatorCredentials;
import io.cdap.plugin.salesforce.parser.SalesforceQueryParser;
import io.cdap.plugin.salesforce.plugin.OAuthInfo;
import org.eclipse.jetty.client.ContentResponse;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Import the SslContextFactory class to configure SSL/TLS support for the HttpClient.

Suggested change
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.util.ssl.SslContextFactory;

import org.eclipse.jetty.util.ssl.SslContextFactory;

import java.io.IOException;
import java.net.HttpURLConnection;
Expand Down Expand Up @@ -152,8 +151,7 @@ public static QueryPlanResponse getQueryPlan(
SalesforceConstants.API_VERSION,
URLEncoder.encode(query, "UTF-8"));

SslContextFactory sslContextFactory = new SslContextFactory();
HttpClient httpClient = new HttpClient(sslContextFactory);
HttpClient httpClient = new HttpClient();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

In Jetty 12, SslContextFactory is abstract, and the default constructor new HttpClient() does not configure SSL/TLS support. Since Salesforce APIs strictly require HTTPS, using new HttpClient() without an explicit SslContextFactory.Client will cause all requests to fail with an exception (e.g., IllegalStateException: Scheme-specific transport not found: https). To fix this, instantiate HttpClient with an instance of SslContextFactory.Client.

Suggested change
HttpClient httpClient = new HttpClient();
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
HttpClient httpClient = new HttpClient(sslContextFactory);

httpClient.setConnectTimeout(credentials.getConnectTimeout());
if (!Strings.isNullOrEmpty(credentials.getProxyUrl())) {
Authenticator.setProxy(credentials, httpClient);
Expand All @@ -163,12 +161,9 @@ public static QueryPlanResponse getQueryPlan(
httpClient.start();
Request request = httpClient.newRequest(explainUrl)
.method(HttpMethod.GET)
.header(
HttpHeader.AUTHORIZATION,
"Bearer " + oAuthInfo.getAccessToken())
.header(
HttpHeader.CONTENT_TYPE,
"application/json");
.headers(headers -> headers
.put(HttpHeader.AUTHORIZATION, "Bearer " + oAuthInfo.getAccessToken())
.put(HttpHeader.CONTENT_TYPE, "application/json"));
ContentResponse response = request.send();
String responseContent = response.getContentAsString();
if (response.getStatus() != HttpURLConnection.HTTP_OK) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpProxy;
import org.eclipse.jetty.client.ProxyConfiguration;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.client.Request;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Import the SslContextFactory class to configure SSL/TLS support for the HttpClient.

Suggested change
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.util.ssl.SslContextFactory;


import java.net.URI;
import java.net.URISyntaxException;
Expand Down Expand Up @@ -89,8 +88,7 @@ public static OAuthInfo getOAuthInfo(AuthenticatorCredentials credentials) throw
throw new IllegalArgumentException("Grant type cannot be null for OAuth flow to fetch access token.");
}

SslContextFactory sslContextFactory = new SslContextFactory();
HttpClient httpClient = new HttpClient(sslContextFactory);
HttpClient httpClient = new HttpClient();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

In Jetty 12, SslContextFactory is abstract, and the default constructor new HttpClient() does not configure SSL/TLS support. Since Salesforce APIs strictly require HTTPS, using new HttpClient() without an explicit SslContextFactory.Client will cause all requests to fail with an exception (e.g., IllegalStateException: Scheme-specific transport not found: https). To fix this, instantiate HttpClient with an instance of SslContextFactory.Client.

Suggested change
HttpClient httpClient = new HttpClient();
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
HttpClient httpClient = new HttpClient(sslContextFactory);

httpClient.setConnectTimeout(credentials.getConnectTimeout());
if (!Strings.isNullOrEmpty(credentials.getProxyUrl())) {
setProxy(credentials, httpClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
import org.cometd.common.JSONContext;
import org.cometd.common.JacksonJSONContextClient;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.client.Request;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Import the SslContextFactory class to configure SSL/TLS support for the HttpClient.

Suggested change
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.util.ssl.SslContextFactory;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -129,10 +128,8 @@ public String getMessage(long timeout, TimeUnit unit) throws InterruptedExceptio
private BayeuxClient getClient(AuthenticatorCredentials credentials) throws Exception {
OAuthInfo oAuthInfo = Authenticator.getOAuthInfo(credentials);

SslContextFactory sslContextFactory = new SslContextFactory();

// Set up a Jetty HTTP client to use with CometD
HttpClient httpClient = new HttpClient(sslContextFactory);
HttpClient httpClient = new HttpClient();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

In Jetty 12, SslContextFactory is abstract, and the default constructor new HttpClient() does not configure SSL/TLS support. Since Salesforce APIs strictly require HTTPS, using new HttpClient() without an explicit SslContextFactory.Client will cause all requests to fail with an exception (e.g., IllegalStateException: Scheme-specific transport not found: https). To fix this, instantiate HttpClient with an instance of SslContextFactory.Client.

Suggested change
HttpClient httpClient = new HttpClient();
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
HttpClient httpClient = new HttpClient(sslContextFactory);

httpClient.setConnectTimeout(CONNECTION_TIMEOUT_MS);
if (!Strings.isNullOrEmpty(credentials.getProxyUrl())) {
Authenticator.setProxy(credentials, httpClient);
Expand All @@ -146,15 +143,15 @@ private BayeuxClient getClient(AuthenticatorCredentials credentials) throws Exce
Map<String, Object> transportOptions = new HashMap<>();
transportOptions.put(ClientTransport.JSON_CONTEXT_OPTION, jsonContext);

// Adds the OAuth header in LongPollingTransport
LongPollingTransport transport = new LongPollingTransport(
transportOptions, httpClient) {
// Adds the OAuth header for all CometD requests
httpClient.getRequestListeners().addListener(new Request.Listener() {
@Override
protected void customize(Request exchange) {
super.customize(exchange);
exchange.header("Authorization", "OAuth " + oAuthInfo.getAccessToken());
public void onBegin(Request request) {
request.headers(headers -> headers.put("Authorization", "OAuth " + oAuthInfo.getAccessToken()));
}
};
});

LongPollingTransport transport = new LongPollingTransport(transportOptions, httpClient);

// Now set up the Bayeux client itself
return new BayeuxClient(oAuthInfo.getInstanceURL() + DEFAULT_PUSH_ENDPOINT, transport);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@
import io.cdap.plugin.salesforce.authenticator.AuthenticatorCredentials;
import io.cdap.plugin.salesforce.plugin.OAuthInfo;
import io.cdap.plugin.salesforce.plugin.source.batch.util.SalesforceSplitUtil;
import org.eclipse.jetty.client.ContentResponse;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Import the SslContextFactory class to configure SSL/TLS support for the mocked HttpClient.

Suggested change
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.util.ssl.SslContextFactory;

import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -296,13 +295,12 @@ public void getQueryPlan_success_returnsQueryPlanResponse() throws Exception {
PowerMockito.mockStatic(Authenticator.class);
PowerMockito.when(Authenticator.getOAuthInfo(credentials)).thenReturn(oAuthInfo);
HttpClient httpClient = PowerMockito.mock(HttpClient.class);
PowerMockito.whenNew(HttpClient.class).withArguments(Mockito.any(SslContextFactory.class))
PowerMockito.whenNew(HttpClient.class).withNoArguments()
.thenReturn(httpClient);
Comment on lines +298 to 299

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Since SalesforceQueryUtil needs to instantiate HttpClient with SslContextFactory.Client to support HTTPS, update the PowerMockito expectation to match the constructor call with arguments.

Suggested change
PowerMockito.whenNew(HttpClient.class).withNoArguments()
.thenReturn(httpClient);
PowerMockito.whenNew(HttpClient.class).withArguments(Mockito.any(SslContextFactory.Client.class))
.thenReturn(httpClient);

Request request = Mockito.mock(Request.class);
Mockito.when(httpClient.newRequest(Mockito.anyString())).thenReturn(request);
Mockito.when(request.method(Mockito.any(HttpMethod.class))).thenReturn(request);
Mockito.when(request.header(Mockito.any(HttpHeader.class), Mockito.anyString()))
.thenReturn(request);
Mockito.when(request.headers(Mockito.any())).thenReturn(request);
ContentResponse response = Mockito.mock(ContentResponse.class);
Mockito.when(request.send()).thenReturn(response);
Mockito.when(response.getStatus()).thenReturn(HttpURLConnection.HTTP_OK);
Expand Down
Loading