diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java index a4c5557dff..bd4e655adb 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/restApiUtils/helpers/RestAPIActivateUtils.java @@ -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, diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java index 5df68082fe..2db27eec9f 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java @@ -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() {} diff --git a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java index 51645d7705..f485d26189 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java @@ -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, diff --git a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java index 12d4d27e7d..1a189d851b 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/test/java/com/external/plugins/RestApiPluginTest.java @@ -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 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 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 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 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(); + } }