Skip to content
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.

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

fix(http): fix java.lang.IllegalStateException: STREAMED #5367

Merged
merged 13 commits into from
Aug 18, 2023
1 change: 1 addition & 0 deletions framework/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ dependencies {
compile group: 'org.pf4j', name: 'pf4j', version: '2.5.0'

testImplementation group: 'org.springframework', name: 'spring-test', version: '5.2.0.RELEASE'
testImplementation group: 'org.springframework', name: 'spring-web', version: '5.2.0.RELEASE'

compile group: 'org.zeromq', name: 'jeromq', version: '0.5.3'
compile project(":chainbase")
Expand Down
59 changes: 48 additions & 11 deletions framework/src/main/java/org/tron/core/services/http/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,22 @@
import com.google.protobuf.GeneratedMessageV3;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.Message;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.lang.reflect.Constructor;
import java.math.BigDecimal;
import java.nio.charset.Charset;
import java.security.InvalidParameterException;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.bouncycastle.util.encoders.Hex;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.util.StringUtil;
import org.tron.api.GrpcAPI;
import org.tron.api.GrpcAPI.BlockList;
Expand Down Expand Up @@ -70,6 +73,8 @@ 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) {
JSONObject jsonObject = new JSONObject();
Expand Down Expand Up @@ -498,16 +503,7 @@ public static void printAccount(Account reply, HttpServletResponse response, Boo
public static byte[] getAddress(HttpServletRequest request) throws Exception {
byte[] address = null;
String addressParam = "address";
String addressStr = request.getParameter(addressParam);
if (StringUtils.isBlank(addressStr)) {
String input = request.getReader().lines()
.collect(Collectors.joining(System.lineSeparator()));
Util.checkBodySize(input);
JSONObject jsonObject = JSON.parseObject(input);
if (jsonObject != null) {
addressStr = jsonObject.getString(addressParam);
}
}
String addressStr = checkGetParam(request, addressParam);
Copy link
Contributor

Choose a reason for hiding this comment

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

getParam or checkAndGetParam ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use checkGetParam as the method name, because there are some checking judgments

if (StringUtils.isNotBlank(addressStr)) {
if (StringUtils.startsWith(addressStr, Constant.ADD_PRE_FIX_STRING_MAINNET)) {
address = Hex.decode(addressStr);
Expand All @@ -518,6 +514,47 @@ public static byte[] getAddress(HttpServletRequest request) throws Exception {
return address;
}

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);
}
if (HttpMethod.POST.toString().toUpperCase().equals(method)) {
String contentType = request.getContentType();
if (StringUtils.isBlank(contentType)) {
halibobo1205 marked this conversation as resolved.
Show resolved Hide resolved
return null;
}
if (APPLICATION_JSON.toLowerCase().contains(contentType)) {
value = getRequestValue(request);
if (StringUtils.isBlank(value)) {
return null;
}

JSONObject jsonObject = JSON.parseObject(value);
if (jsonObject != null) {
return jsonObject.getString(key);
}
} else if (APPLICATION_FORM_URLENCODED.toLowerCase().contains(contentType)) {
return request.getParameter(key);
} else {
return null;
}
}
return value;
}

public static String getRequestValue(HttpServletRequest request) throws IOException {
BufferedReader reader = new BufferedReader(new InputStreamReader(request.getInputStream()));
String line;
StringBuilder sb = new StringBuilder();
while ((line = reader.readLine()) != null) {
sb.append(line);
}
return sb.toString();
}

public static List<Log> convertLogAddressToTronAddress(TransactionInfo transactionInfo) {
List<Log> newLogList = new ArrayList<>();

Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

For historical reasons, this logic does not have a unit test.

Better add comprehensive unit tests for all situations after changing the whole logic of the param extraction to make sure all the new logic works well, such as: good requests for both form&json

Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package org.tron.core.services.http;

import static org.junit.Assert.assertEquals;

import com.alibaba.fastjson.JSONObject;

import java.io.UnsupportedEncodingException;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;

public class GetBrokerageServletTest {

private MockHttpServletRequest request;
private MockHttpServletResponse response;

@Before
public void setUp() {
request = new MockHttpServletRequest();
request.setMethod("POST");
request.setContentType("application/x-www-form-urlencoded");
request.setCharacterEncoding("UTF-8");

response = new MockHttpServletResponse();
}

@Test
public void getBrokerageTest() {
request.addParameter("address", "");
GetRewardServlet getRewardServlet = new GetRewardServlet();
getRewardServlet.doPost(request, response);
try {
String contentAsString = response.getContentAsString();
JSONObject result = JSONObject.parseObject(contentAsString);
int reward = (int)result.get("reward");
Assert.assertEquals(0, reward);
String content = (String) result.get("Error");
Assert.assertNull(content);
} catch (UnsupportedEncodingException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Assert.fail(); is a good choice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not expecting an exception to be thrown; the end result of this use case is a normal return of the

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that even if an UnsupportedEncodingException is thrown, it's still normal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using a statement when pointing out a problem is a good way to respond; if you use a rhetorical question, I can't be sure of your intentions and it doesn't help to accurately describe the problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The point of this use case is not to test for the UnsupportedEncodingException, but rather to determine the value of null.

Copy link
Contributor

Choose a reason for hiding this comment

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

Assert.fail() is added where it is not possible to reach in the expectation, once it is reached, indicates that the test has failed, the result is not as expected.

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.tron.core.services.http;

import com.alibaba.fastjson.JSONObject;

import java.io.UnsupportedEncodingException;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;

public class GetRewardServletTest {

private MockHttpServletRequest request;
private MockHttpServletResponse response;

@Before
public void setUp() {
request = new MockHttpServletRequest();
request.setMethod("POST");
request.setContentType("application/x-www-form-urlencoded");
request.setCharacterEncoding("UTF-8");

response = new MockHttpServletResponse();
}

@Test
public void getRewardTest() {
request.addParameter("address", "");
GetRewardServlet getRewardServlet = new GetRewardServlet();
getRewardServlet.doPost(request, response);
try {
String contentAsString = response.getContentAsString();
JSONObject result = JSONObject.parseObject(contentAsString);
int reward = (int)result.get("reward");
Assert.assertEquals(0, reward);
String content = (String) result.get("Error");
Assert.assertNull(content);
} catch (UnsupportedEncodingException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please test all cases of checkGetParam.

Copy link
Contributor Author

@forfreeday forfreeday Aug 14, 2023

Choose a reason for hiding this comment

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

checkGetParam is business-independent and does not need to be validated.

}
}
}