Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: for api-redirection (#27720)

Browse Source 
## Description
> Fixes the API redirection problem where forwarding location only has
uri path

This Pr adds null pointer checks for host address, adds stack trace in
logging, and fixes the problem of erroring out when the called endpoint
redirection headers don't have full formed forwarding location


#### PR fixes following issue(s)
Fixes #25408 

#### Type of change
- Bug fix (non-breaking change which fixes an issue)

#### How Has This Been Tested?
- [x] Manual
- [ ] JUnit

#### Test Plan
1. Tried out with the endpoint of the API hosted on Manish's ngrok, GET
and POST requests fail with the java null pointer exception. However, on
this DP, both GET and POST requests work.
2. Tried out Sanity tests with other end points on Rest API
3. Did Sanity checks on GraphQL
4. Saved Rest API and GraphQL as datasource and ensured that works fine
too
5. Exported this app, imported the same and ran the API to check for
functionality

## Checklist:
#### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [x] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [x] Test plan has been peer reviewed by project stakeholders and other
QA members
- [x] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
This commit is contained in:
Manish Kumar 2023-10-11 21:54:00 +05:30 committed by GitHub
parent 53d80492e8
commit b31fcb11da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 211 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java vendored
View File

@ -24,6 +24,7 @@ import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.MediaType;
import org.springframework.http.client.reactive.ClientHttpRequest;
import org.springframework.util.CollectionUtils;
import org.springframework.web.reactive.function.BodyInserter;
import org.springframework.web.reactive.function.client.ClientResponse;
import org.springframework.web.reactive.function.client.ExchangeStrategies;
@ -36,7 +37,6 @@ import reactor.netty.resources.ConnectionProvider;
import javax.crypto.SecretKey;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.time.Instant;
@ -96,7 +96,12 @@ public class RestAPIActivateUtils {
actionExecutionRequest, isBodySentWithApiRequest));
result.setStatusCode(statusCode.toString());
result.setIsExecutionSuccess(statusCode.is2xxSuccessful());
// if something has moved permanently should we mark it as an execution failure?
// here marking a redirection as an execution success if the url has moved permanently without a
// forwarding Location
boolean isExecutionSuccess = statusCode.is2xxSuccessful() || statusCode.is3xxRedirection();
result.setIsExecutionSuccess(isExecutionSuccess);
// Convert the headers into json tree to store in the results
String headerInJsonString;
@ -191,19 +196,18 @@ public class RestAPIActivateUtils {
.exchange()
.flatMap(response -> {
if (response.statusCode().is3xxRedirection()) {
// if there is no redirect location then we should just return the response
if (CollectionUtils.isEmpty(response.headers().header(HttpHeaders.LOCATION))) {
return Mono.just(response);
}
String redirectUrl =
response.headers().header("Location").get(0);
/**
* TODO
* In case the redirected URL is not absolute (complete), create the new URL using the relative path
* This particular scenario is seen in the URL : https://rickandmortyapi.com/api/character
* It redirects to partial URI : /api/character/
* In this scenario we should convert the partial URI to complete URI
*/
final URI redirectUri;
try {
redirectUri = new URI(redirectUrl);
} catch (URISyntaxException e) {
redirectUri = createRedirectUrl(redirectUrl, uri);
} catch (IllegalArgumentException e) {
return Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, e));
}
@ -213,6 +217,10 @@ public class RestAPIActivateUtils {
});
}
public URI createRedirectUrl(String redirectUrl, URI originalUrl) {
return originalUrl.resolve(redirectUrl);
}
public WebClient getWebClient(
WebClient.Builder webClientBuilder,
APIConnection apiConnection,

16
app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java
View File

@ -1,5 +1,7 @@
package com.appsmith.util;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
import io.netty.resolver.AddressResolver;
import io.netty.resolver.AddressResolverGroup;
import io.netty.resolver.InetNameResolver;
@ -9,6 +11,7 @@ import io.netty.util.concurrent.Promise;
import io.netty.util.internal.SocketUtils;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.client.reactive.ReactorClientHttpConnector;
import org.springframework.util.StringUtils;
import org.springframework.web.reactive.function.client.ExchangeFilterFunction;
import org.springframework.web.reactive.function.client.WebClient;
import reactor.core.publisher.Mono;
@ -29,10 +32,15 @@ public class WebClientUtils {
public static final String HOST_NOT_ALLOWED = "Host not allowed.";
public static final ExchangeFilterFunction IP_CHECK_FILTER = ExchangeFilterFunction.ofRequestProcessor(
request -> DISALLOWED_HOSTS.contains(request.url().getHost())
? Mono.error(new UnknownHostException(HOST_NOT_ALLOWED))
: Mono.just(request));
public static final ExchangeFilterFunction IP_CHECK_FILTER = ExchangeFilterFunction.ofRequestProcessor(request -> {
if (!StringUtils.hasText(request.url().getHost())) {
return Mono.error(new AppsmithPluginException(
AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR, "Requested url host is null or empty"));
}
return DISALLOWED_HOSTS.contains(request.url().getHost())
? Mono.error(new UnknownHostException(HOST_NOT_ALLOWED))
: Mono.just(request);
});
private WebClientUtils() {}

5
app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java vendored
View File

@ -208,6 +208,11 @@ public class RestApiPlugin extends BasePlugin {
errorResult.setRequest(requestCaptureFilter.populateRequestFields(
actionExecutionRequest, isBodySentWithApiRequest));
errorResult.setIsExecutionSuccess(false);
log.debug(
"An error has occurred while trying to run the API query for url: {}, path : {}",
datasourceConfiguration.getUrl(),
actionConfiguration.getPath());
error.printStackTrace();
if (!(error instanceof AppsmithPluginException)) {
error = new AppsmithPluginException(
RestApiPluginError.API_EXECUTION_FAILED,

175
app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java vendored
View File

@ -3,6 +3,7 @@ package com.external.plugins;
import com.appsmith.external.datatypes.ClientDataType;
import com.appsmith.external.dtos.ExecuteActionDTO;
import com.appsmith.external.dtos.ParamProperty;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
import com.appsmith.external.helpers.PluginUtils;
import com.appsmith.external.helpers.restApiUtils.connections.APIConnection;
import com.appsmith.external.helpers.restApiUtils.helpers.HintMessageUtils;
@ -1919,4 +1920,178 @@ public class RestApiPluginTest {
})
.verifyComplete();
}
@Test
public void testRedirectionSuccessWithLocationHeaderHavingPath() {
DatasourceConfiguration dsConfig = new DatasourceConfiguration();
String baseUrl = String.format("http://%s:%s", mockEndpoint.getHostName(), mockEndpoint.getPort());
dsConfig.setUrl(baseUrl);
String initialPath = "/start";
String redirectPath = "/redirection";
String responseBody = "this is a successful response";
MockResponse initialResponse =
new MockResponse().setResponseCode(302).addHeader(HttpHeaders.LOCATION, redirectPath);
MockResponse redirectedResponse =
new MockResponse().setResponseCode(200).setBody(responseBody);
mockEndpoint.enqueue(initialResponse);
mockEndpoint.enqueue(redirectedResponse);
ActionConfiguration actionConfig = new ActionConfiguration();
actionConfig.setHttpMethod(HttpMethod.GET);
actionConfig.setPath(initialPath);
Mono<ActionExecutionResult> resultMono =
pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig);
StepVerifier.create(resultMono)
.assertNext(result -> {
assertTrue(result.getIsExecutionSuccess());
assertNotNull(result.getBody());
final RecordedRequest initialRequest;
try {
initialRequest = mockEndpoint.takeRequest(30, TimeUnit.SECONDS);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
assert initialRequest != null;
assertEquals(initialRequest.getPath(), initialPath);
// redirected Request
final RecordedRequest redirectedRequest;
try {
redirectedRequest = mockEndpoint.takeRequest(30, TimeUnit.SECONDS);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
assert redirectedRequest != null;
assertEquals(redirectedRequest.getPath(), redirectPath);
final ActionExecutionRequest request = result.getRequest();
assertEquals(baseUrl + redirectPath, request.getUrl());
assertEquals(HttpMethod.GET, request.getHttpMethod());
assertEquals(result.getBody(), responseBody);
})
.verifyComplete();
}
@Test
public void testRedirectionSuccessWithAddingFullUrlExecution() {
DatasourceConfiguration dsConfig = new DatasourceConfiguration();
String baseUrl = String.format("http://%s:%s", mockEndpoint.getHostName(), mockEndpoint.getPort());
dsConfig.setUrl(baseUrl);
String initialPath = "/start";
String redirectPath = "/redirection";
String responseBody = "this is a successful response";
MockResponse initialResponse =
new MockResponse().setResponseCode(302).addHeader(HttpHeaders.LOCATION, baseUrl + redirectPath);
MockResponse redirectedResponse =
new MockResponse().setResponseCode(200).setBody(responseBody);
mockEndpoint.enqueue(initialResponse);
mockEndpoint.enqueue(redirectedResponse);
ActionConfiguration actionConfig = new ActionConfiguration();
actionConfig.setHttpMethod(HttpMethod.GET);
actionConfig.setPath(initialPath);
Mono<ActionExecutionResult> resultMono =
pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig);
StepVerifier.create(resultMono)
.assertNext(result -> {
assertTrue(result.getIsExecutionSuccess());
assertNotNull(result.getBody());
final RecordedRequest initialRequest;
try {
initialRequest = mockEndpoint.takeRequest(30, TimeUnit.SECONDS);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
assert initialRequest != null;
assertEquals(initialRequest.getPath(), initialPath);
// redirected Request
final RecordedRequest redirectedRequest;
try {
redirectedRequest = mockEndpoint.takeRequest(30, TimeUnit.SECONDS);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
assert redirectedRequest != null;
assertEquals(redirectedRequest.getPath(), redirectPath);
final ActionExecutionRequest request = result.getRequest();
assertEquals(baseUrl + redirectPath, request.getUrl());
assertEquals(HttpMethod.GET, request.getHttpMethod());
assertEquals(result.getBody(), responseBody);
})
.verifyComplete();
}
@Test
public void testExecutionSuccessWhenRedirectionEndsWithoutALocationHeader() {
DatasourceConfiguration dsConfig = new DatasourceConfiguration();
String baseUrl = String.format("http://%s:%s", mockEndpoint.getHostName(), mockEndpoint.getPort());
dsConfig.setUrl(baseUrl);
String path = "/start";
String responseBody = "this is a successful response, even when it ends with a redirection";
MockResponse initialResponse = new MockResponse().setResponseCode(302).setBody(responseBody);
mockEndpoint.enqueue(initialResponse);
ActionConfiguration actionConfig = new ActionConfiguration();
actionConfig.setHttpMethod(HttpMethod.GET);
actionConfig.setPath(path);
Mono<ActionExecutionResult> resultMono =
pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig);
StepVerifier.create(resultMono)
.assertNext(result -> {
assertTrue(result.getIsExecutionSuccess());
assertNotNull(result.getBody());
final RecordedRequest finalRequest;
try {
finalRequest = mockEndpoint.takeRequest(30, TimeUnit.SECONDS);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
assert finalRequest != null;
assertEquals(finalRequest.getPath(), path);
final ActionExecutionRequest request = result.getRequest();
assertEquals(baseUrl + path, request.getUrl());
assertEquals(HttpMethod.GET, request.getHttpMethod());
assertEquals(result.getBody(), responseBody);
assertTrue(result.getIsExecutionSuccess());
})
.verifyComplete();
}
@Test
public void testEmptyHostErrorMessage() {
DatasourceConfiguration dsConfig = new DatasourceConfiguration();
dsConfig.setUrl(" ");
String path = "/start";
ActionConfiguration actionConfig = new ActionConfiguration();
actionConfig.setHttpMethod(HttpMethod.GET);
actionConfig.setPath(path);
Mono<ActionExecutionResult> resultMono =
pluginExecutor.executeParameterized(null, new ExecuteActionDTO(), dsConfig, actionConfig);
StepVerifier.create(resultMono)
.assertNext(actionExecutionResult -> {
assertFalse(actionExecutionResult.getIsExecutionSuccess());
assertEquals(
actionExecutionResult.getStatusCode(),
AppsmithPluginError.PLUGIN_DATASOURCE_ARGUMENT_ERROR.getAppErrorCode());
})
.verifyComplete();
}
}