Skip to content

feat(http): optimize parameter parsing #5483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
halibobo1205 merged 6 commits into from
Sep 14, 2023
Merged

feat(http): optimize parameter parsing #5483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
48585eb
feat(Util): fix util
Sep 12, 2023
be8f186
feat(Util): optimize judgment logic
Sep 13, 2023
e6a5c68
test(case): add GetRewardServletTest and GetBrokerageServletTest test…
Sep 13, 2023
5292497
Merge branch 'release_v4.7.3' into feature/fix_utils
Sep 13, 2023
a241aa3
test(case): fix code
Sep 13, 2023
54884dc
feature(Util): remove 'content-type' judgment
Sep 14, 2023
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
15 changes: 6 additions & 9 deletions framework/src/main/java/org/tron/core/services/http/Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.commons.lang3.StringUtils;
import org.bouncycastle.util.encoders.Hex;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.util.MultiMap;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.UrlEncoded;
Expand Down Expand Up @@ -78,8 +79,6 @@ public class Util {
public static final String FUNCTION_SELECTOR = "function_selector";
public static final String FUNCTION_PARAMETER = "parameter";
public static final String CALL_DATA = "data";
public static final String APPLICATION_FORM_URLENCODED = "application/x-www-form-urlencoded";
public static final String APPLICATION_JSON = "application/json";

public static String printTransactionFee(String transactionFee) {
Copy link
Copy Markdown
Contributor

@tomatoishealthy tomatoishealthy Sep 13, 2023

Choose a reason for hiding this comment

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

Better add unit test

JSONObject jsonObject = new JSONObject();
Expand Down Expand Up @@ -525,7 +524,6 @@ public static byte[] getAddress(HttpServletRequest request) throws Exception {

private static String checkGetParam(HttpServletRequest request, String key) throws Exception {
String method = request.getMethod();
String value = null;

if (HttpMethod.GET.toString().toUpperCase().equalsIgnoreCase(method)) {
return request.getParameter(key);
Expand All @@ -535,8 +533,10 @@ private static String checkGetParam(HttpServletRequest request, String key) thro
if (StringUtils.isBlank(contentType)) {
return null;
}
if (APPLICATION_JSON.toLowerCase().contains(contentType)) {
Copy link
Copy Markdown
Contributor

@lurais lurais Sep 13, 2023

Choose a reason for hiding this comment

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

Should the contentType to lower case too ?

Copy link
Copy Markdown
Contributor Author

@forfreeday forfreeday Sep 13, 2023

Choose a reason for hiding this comment

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

defaults to lowercase.

value = getRequestValue(request);
if (contentType.contains(MimeTypes.Type.FORM_ENCODED.asString())) {
Copy link
Copy Markdown
Contributor

@lurais lurais Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Shall we add some explanation about this change?

return request.getParameter(key);
} else {
String value = getRequestValue(request);
if (StringUtils.isBlank(value)) {
return null;
}
Expand All @@ -545,13 +545,10 @@ private static String checkGetParam(HttpServletRequest request, String key) thro
if (jsonObject != null) {
return jsonObject.getString(key);
}
} else if (APPLICATION_FORM_URLENCODED.toLowerCase().contains(contentType)) {
return request.getParameter(key);
} else {
return null;
}
}
return value;
return null;
}

public static String getRequestValue(HttpServletRequest request) throws IOException {
Expand Down
19 changes: 19 additions & 0 deletions framework/src/test/java/org/tron/core/services/http/GetBrokerageServletTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,25 @@ public void getBrokerageValueByJsonTest() {
}
}


@Test
public void getBrokerageByJsonUTF8Test() {
int expect = 20;
String jsonParam = "{\"address\": \"27VZHn9PFZwNh7o2EporxmLkpe157iWZVkh\"}";
MockHttpServletRequest request = createRequest("application/json; charset=utf-8");
request.setContent(jsonParam.getBytes());
MockHttpServletResponse response = new MockHttpServletResponse();
getBrokerageServlet.doPost(request, response);
try {
String contentAsString = response.getContentAsString();
JSONObject result = JSONObject.parseObject(contentAsString);
int brokerage = (int)result.get("brokerage");
Assert.assertEquals(expect, brokerage);
} catch (UnsupportedEncodingException e) {
Assert.fail(e.getMessage());
}
}

@Test
public void getBrokerageValueTest() {
int expect = 20;
Expand Down
24 changes: 22 additions & 2 deletions framework/src/test/java/org/tron/core/services/http/GetRewardServletTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
import java.io.UnsupportedEncodingException;
import javax.annotation.Resource;

import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
Expand All @@ -22,6 +24,7 @@
import org.tron.core.service.MortgageService;
import org.tron.core.store.DelegationStore;

@Slf4j
public class GetRewardServletTest extends BaseTest {

@Resource
Expand Down Expand Up @@ -52,6 +55,7 @@ public MockHttpServletRequest createRequest(String contentType) {
return request;
}

@Before
public void init() {
manager.getDynamicPropertiesStore().saveChangeDelegation(1);
byte[] sr = decodeFromBase58Check("27VZHn9PFZwNh7o2EporxmLkpe157iWZVkh");
Expand All @@ -61,7 +65,6 @@ public void init() {

@Test
public void getRewardValueByJsonTest() {
init();
int expect = 138181;
String jsonParam = "{\"address\": \"27VZHn9PFZwNh7o2EporxmLkpe157iWZVkh\"}";
MockHttpServletRequest request = createRequest("application/json");
Expand All @@ -78,9 +81,26 @@ public void getRewardValueByJsonTest() {
}
}

@Test
public void getRewardByJsonUTF8Test() {
int expect = 138181;
String jsonParam = "{\"address\": \"27VZHn9PFZwNh7o2EporxmLkpe157iWZVkh\"}";
MockHttpServletRequest request = createRequest("application/json; charset=utf-8");
MockHttpServletResponse response = new MockHttpServletResponse();
request.setContent(jsonParam.getBytes());
try {
getRewardServlet.doPost(request, response);
String contentAsString = response.getContentAsString();
JSONObject result = JSONObject.parseObject(contentAsString);
int reward = (int)result.get("reward");
Assert.assertEquals(expect, reward);
} catch (UnsupportedEncodingException e) {
Assert.fail(e.getMessage());
}
}

@Test
public void getRewardValueTest() {
init();
int expect = 138181;
MockHttpServletRequest request = createRequest("application/x-www-form-urlencoded");
MockHttpServletResponse response = new MockHttpServletResponse();
Expand Down