From 450950d68f45b4683a3376767ffd66f4e1e8c0d3 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Thu, 24 Feb 2022 13:58:26 +0330 Subject: [PATCH] fix: curl import tries to guess the content-type from the body if content-type header is not specified (#11295) * fix: content-type is empty when it's not specified. * feat: try to guess the content type json or form-urlencoded * fix: broken test for curl with `--data-urlencode` parameter * fix: fix guessing urlEncodedPattern * fix: fix broken tests * fix: Fixed and improved the code formatting --- .../services/CurlImporterServiceImpl.java | 15 +++-- .../ce/CurlImporterServiceCEImpl.java | 58 +++++++++++++++---- .../services/CurlImporterServiceTest.java | 55 +++++++----------- 3 files changed, 76 insertions(+), 52 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/CurlImporterServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/CurlImporterServiceImpl.java index 7ac7089906..12d44abc42 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/CurlImporterServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/CurlImporterServiceImpl.java @@ -2,6 +2,7 @@ package com.appsmith.server.services; import com.appsmith.server.helpers.ResponseUtils; import com.appsmith.server.services.ce.CurlImporterServiceCEImpl; +import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -9,11 +10,13 @@ import org.springframework.stereotype.Service; @Service public class CurlImporterServiceImpl extends CurlImporterServiceCEImpl implements CurlImporterService { - public CurlImporterServiceImpl(PluginService pluginService, - LayoutActionService layoutActionService, - NewPageService newPageService, - ResponseUtils responseUtils) { - - super(pluginService, layoutActionService, newPageService, responseUtils); + public CurlImporterServiceImpl( + PluginService pluginService, + LayoutActionService layoutActionService, + NewPageService newPageService, + ResponseUtils responseUtils, + ObjectMapper objectMapper + ) { + super(pluginService, layoutActionService, newPageService, responseUtils, objectMapper); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CurlImporterServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CurlImporterServiceCEImpl.java index 74ff7745b6..89ca00d09b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CurlImporterServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CurlImporterServiceCEImpl.java @@ -15,6 +15,9 @@ import com.appsmith.server.services.BaseApiImporter; import com.appsmith.server.services.LayoutActionService; import com.appsmith.server.services.NewPageService; import com.appsmith.server.services.PluginService; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; import org.apache.http.NameValuePair; @@ -32,6 +35,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Base64; import java.util.List; +import java.util.regex.Pattern; import static com.appsmith.server.acl.AclPermission.MANAGE_PAGES; @@ -52,15 +56,20 @@ public class CurlImporterServiceCEImpl extends BaseApiImporter implements CurlIm private final LayoutActionService layoutActionService; private final ResponseUtils responseUtils; private final NewPageService newPageService; + private final ObjectMapper objectMapper; - public CurlImporterServiceCEImpl(PluginService pluginService, - LayoutActionService layoutActionService, - NewPageService newPageService, - ResponseUtils responseUtils) { + public CurlImporterServiceCEImpl( + PluginService pluginService, + LayoutActionService layoutActionService, + NewPageService newPageService, + ResponseUtils responseUtils, + ObjectMapper objectMapper + ) { this.pluginService = pluginService; this.layoutActionService = layoutActionService; this.newPageService = newPageService; this.responseUtils = responseUtils; + this.objectMapper = objectMapper; } @Override @@ -329,7 +338,12 @@ public class CurlImporterServiceCEImpl extends BaseApiImporter implements CurlIm } else if ("--data-urlencode".equals(state)) { // The `token` is next to `--data-urlencode`. - dataParts.add(token); + // ignore the '=' at the start as the curl document says https://curl.se/docs/manpage.html#--data-urlencode + if (token.startsWith("=")) { + dataParts.add(token.substring(1)); + } else { + dataParts.add(token); + } } else if (ARG_FORM.equals(state)) { // The token is next to --form @@ -371,13 +385,11 @@ public class CurlImporterServiceCEImpl extends BaseApiImporter implements CurlIm } - if (contentType == null && !dataParts.isEmpty()) { - contentType = MediaType.APPLICATION_FORM_URLENCODED_VALUE; - headers.add(new Property(HttpHeaders.CONTENT_TYPE, contentType)); - - } else if (contentType == null && !formParts.isEmpty()) { - contentType = MediaType.MULTIPART_FORM_DATA_VALUE; - headers.add(new Property(HttpHeaders.CONTENT_TYPE, contentType)); + if (contentType == null) { + contentType = guessTheContentType(dataParts, formParts); + if (contentType != null) { + headers.add(new Property(HttpHeaders.CONTENT_TYPE, contentType)); + } } if (!headers.isEmpty()) { @@ -423,6 +435,28 @@ public class CurlImporterServiceCEImpl extends BaseApiImporter implements CurlIm return action; } + private String guessTheContentType(List dataParts, List formParts) { + if (!dataParts.isEmpty()) { + final String data = dataParts.get(0); + final Pattern urlEncodedPattern = Pattern.compile("([A-Za-z0-9%._\\-/]+=[^\\s]+)"); + // if it's form url encoded? + if (urlEncodedPattern.matcher(data).matches()) { + return MediaType.APPLICATION_FORM_URLENCODED_VALUE; + } else { + // or if it's JSON + try { + objectMapper.readTree(data); + return MediaType.APPLICATION_JSON_VALUE; + } catch (JsonProcessingException e) { + // ignore exception it's not JSON + } + } + } else if (!formParts.isEmpty()) { + return MediaType.MULTIPART_FORM_DATA_VALUE; + } + return null; + } + private void trySaveURL(ActionDTO action, String token) throws MalformedURLException, URISyntaxException { // If the URL appears to not have a protocol set, prepend the `https` protocol. if (!token.matches("\\w+://.*")) { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/CurlImporterServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/CurlImporterServiceTest.java index da2e617f41..13b09234ee 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/CurlImporterServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/CurlImporterServiceTest.java @@ -200,12 +200,12 @@ public class CurlImporterServiceTest { Mono branchedPageMono = defaultPageMono .flatMap(defaultPage -> - newPageService.findById(branchedPageId, AclPermission.MANAGE_PAGES) - .flatMap(newPage -> { - newPage.setDefaultResources(defaultPage.getDefaultResources()); - newPage.getDefaultResources().setBranchName("testBranch"); - return newPageService.save(newPage); - }) + newPageService.findById(branchedPageId, AclPermission.MANAGE_PAGES) + .flatMap(newPage -> { + newPage.setDefaultResources(defaultPage.getDefaultResources()); + newPage.getDefaultResources().setBranchName("testBranch"); + return newPageService.save(newPage); + }) ) .cache(); @@ -311,33 +311,24 @@ public class CurlImporterServiceTest { ); assertMethod(action, HttpMethod.POST); assertUrl(action, "http://loc"); - assertEmptyBody(action); - assertBodyFormData( - action, - new Property("", "all of this exactly, but url encoded ") - ); + assertBody(action, "all of this exactly, but url encoded "); + assertEmptyBodyFormData(action); action = curlImporterService.curlToAction( "curl --data-urlencode 'spaced name=all of this exactly, but url encoded' http://loc" ); assertMethod(action, HttpMethod.POST); assertUrl(action, "http://loc"); - assertEmptyBody(action); - assertBodyFormData( - action, - new Property("spaced name", "all of this exactly, but url encoded") - ); + assertBody(action, "spaced name=all of this exactly, but url encoded"); + assertEmptyBodyFormData(action); action = curlImporterService.curlToAction( "curl --data-urlencode 'awesome=details, all of this exactly, but url encoded' http://loc" ); assertMethod(action, HttpMethod.POST); assertUrl(action, "http://loc"); - assertEmptyBody(action); - assertBodyFormData( - action, - new Property("awesome", "details, all of this exactly, but url encoded") - ); + assertBody(action, "awesome=details, all of this exactly, but url encoded"); + assertEmptyBodyFormData(action); } @Test @@ -616,7 +607,7 @@ public class CurlImporterServiceTest { assertMethod(action, HttpMethod.POST); assertUrl(action, "https://api.sloths.com"); assertEmptyPath(action); - assertHeaders(action, new Property("Content-Type", "application/x-www-form-urlencoded")); + assertHeaders(action, new Property("Content-Type", "application/x-www-form-urlencoded")); assertEmptyBody(action); assertBodyFormData( action, @@ -702,12 +693,9 @@ public class CurlImporterServiceTest { assertMethod(action, HttpMethod.POST); assertUrl(action, "http://dummy.restapiexample.com"); assertPath(action, "/api/v1/create"); - assertHeaders(action, new Property("Content-Type", "application/x-www-form-urlencoded")); - assertEmptyBody(action); - assertBodyFormData( - action, - new Property("{\"name\":\"test\",\"salary\":\"123\",\"age\":\"23\"}", "") - ); + assertHeaders(action, new Property("Content-Type", "application/json")); + assertBody(action, "{\"name\":\"test\",\"salary\":\"123\",\"age\":\"23\"}"); + assertEmptyBodyFormData(action); } @Test @@ -778,12 +766,8 @@ public class CurlImporterServiceTest { assertMethod(action, HttpMethod.POST); assertUrl(action, "http://httpbin.org"); assertPath(action, "/post"); - assertHeaders(action, new Property("Content-Type", "application/x-www-form-urlencoded")); - assertEmptyBody(action); - assertBodyFormData( - action, - new Property("a\\n", "") - ); + assertBody(action, "a\\n"); + assertEmptyBodyFormData(action); } @Test @@ -848,6 +832,9 @@ public class CurlImporterServiceTest { private static void assertEmptyBody(ActionDTO action) { assertThat(action.getActionConfiguration().getBody()).isNullOrEmpty(); } + private static void assertEmptyBodyFormData(ActionDTO action) { + assertThat(action.getActionConfiguration().getBodyFormData()).isNullOrEmpty(); + } private static void assertBody(ActionDTO action, String body) { assertThat(action.getActionConfiguration().getBody()).isEqualTo(body);