Fix the parsing issue in large post request for transport-simple-http

[jasonjoo2010]

Describe what this PR does / why we need it

Please refer to #1253

Improve unit test for HttpEventTask.

Does this pull request fix one issue?

Yes.

Describe how you did it

Change single read into iteration.

Describe how to verify it

Do post request larger than 10kB.

Special notes for reviews

Nope

[codecov-io]

Codecov Report

Merging #1255 into master will decrease coverage by 0.21%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1255      +/-   ##
============================================
- Coverage     43.31%   43.09%   -0.22%     
- Complexity     1581     1629      +48     
============================================
  Files           338      350      +12     
  Lines          9905    10304     +399     
  Branches       1337     1389      +52     
============================================
+ Hits           4290     4441     +151     
- Misses         5089     5310     +221     
- Partials        526      553      +27

Impacted Files	Coverage Δ	Complexity Δ
...n/java/com/alibaba/csp/sentinel/log/RecordLog.java	35.89% <0%> (-56.42%)	5% <0%> (ø)
.../sentinel/adapter/dubbo/DubboAppContextFilter.java	80% <0%> (-20%)	2% <0%> (-1%)
...entinel/adapter/dubbo/BaseSentinelDubboFilter.java	77.77% <0%> (-4.84%)	5% <0%> (ø)
...ava/com/alibaba/csp/sentinel/node/ClusterNode.java	96.55% <0%> (-3.45%)	10% <0%> (-1%)
...a/csp/sentinel/slots/statistic/base/LeapArray.java	67.32% <0%> (-2.98%)	33% <0%> (-1%)
...java/com/alibaba/csp/sentinel/context/Context.java	64.7% <0%> (-2.95%)	15% <0%> (ø)
.../adapter/spring/webmvc/SentinelWebInterceptor.java	40% <0%> (-1.18%)	3% <0%> (+1%)
...com/alibaba/csp/sentinel/log/CommandCenterLog.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
...ba/csp/sentinel/adapter/sofa/rpc/SofaRpcUtils.java	100% <0%> (ø)	7% <0%> (?)
...ter/sofa/rpc/fallback/SofaRpcFallbackRegistry.java	100% <0%> (ø)	5% <0%> (?)
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b136848...1ed3525. Read the comment docs.

[jasonjoo2010]

It's a quite old bug and simple pr so i think we should get it done soon.

Refer to #1253

[sczyh30]

I'll test the PR this weekend :)

[sczyh30]

I've tested with the code and when I was publishing the rule via the dashboard, an error occurred:

2020-03-09 12:32:02.377 WARNING [SimpleHttpCommandCenter] CommandCenter error
java.net.SocketTimeoutException: Read timed out
	at java.net.SocketInputStream.socketRead0(Native Method)
	at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
	at java.net.SocketInputStream.read(SocketInputStream.java:171)
	at java.net.SocketInputStream.read(SocketInputStream.java:141)
	at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
	at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
	at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
	at java.io.InputStreamReader.read(InputStreamReader.java:184)
	at java.io.BufferedReader.fill(BufferedReader.java:161)
	at java.io.BufferedReader.read1(BufferedReader.java:212)
	at java.io.BufferedReader.read(BufferedReader.java:286)
	at java.io.Reader.read(Reader.java:140)
	at com.alibaba.csp.sentinel.transport.command.http.HttpEventTask.consumePostBody(HttpEventTask.java:215)
	at com.alibaba.csp.sentinel.transport.command.http.HttpEventTask.processPostRequest(HttpEventTask.java:147)
	at com.alibaba.csp.sentinel.transport.command.http.HttpEventTask.run(HttpEventTask.java:83)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:266)
	at java.util.concurrent.FutureTask.run(FutureTask.java)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

The error occurred in the second time of reading:

Sentinel/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/command/http/HttpEventTask.java

Lines 210 to 217 in 0c9150a

	protected static void consumePostBody(Reader in, CommandRequest request) throws IOException {
	StringBuilder builder = new StringBuilder();
	char[] buf = new char[1024];
	while (true) {
	int read = in.read(buf);
	if (read <= 0) {
	break;
	}

It seems that the SocketInputStream will wait until timeout, rather than for EOF. Maybe we need to check it via in.ready() or other approaches? @jasonjoo2010

[jasonjoo2010]

I've tested with the code and when I was publishing the rule via the dashboard, an error occurred:

2020-03-09 12:32:02.377 WARNING [SimpleHttpCommandCenter] CommandCenter error
java.net.SocketTimeoutException: Read timed out
	at java.net.SocketInputStream.socketRead0(Native Method)
	at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
	at java.net.SocketInputStream.read(SocketInputStream.java:171)
	at java.net.SocketInputStream.read(SocketInputStream.java:141)
	at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
	at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
	at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
	at java.io.InputStreamReader.read(InputStreamReader.java:184)
	at java.io.BufferedReader.fill(BufferedReader.java:161)
	at java.io.BufferedReader.read1(BufferedReader.java:212)
	at java.io.BufferedReader.read(BufferedReader.java:286)
	at java.io.Reader.read(Reader.java:140)
	at com.alibaba.csp.sentinel.transport.command.http.HttpEventTask.consumePostBody(HttpEventTask.java:215)
	at com.alibaba.csp.sentinel.transport.command.http.HttpEventTask.processPostRequest(HttpEventTask.java:147)
	at com.alibaba.csp.sentinel.transport.command.http.HttpEventTask.run(HttpEventTask.java:83)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:266)
	at java.util.concurrent.FutureTask.run(FutureTask.java)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

The error occurred in the second time of reading:

Sentinel/sentinel-transport/sentinel-transport-simple-http/src/main/java/com/alibaba/csp/sentinel/transport/command/http/HttpEventTask.java

Lines 210 to 217 in 0c9150a

	protected static void consumePostBody(Reader in, CommandRequest request) throws IOException {
	StringBuilder builder = new StringBuilder();
	char[] buf = new char[1024];
	while (true) {
	int read = in.read(buf);
	if (read <= 0) {
	break;
	}

It seems that the SocketInputStream will wait until timeout, rather than for EOF. Maybe we need to check it via in.ready() or other approaches? @jasonjoo2010

I have restructured it dealing with the Content-Length header. Now it should be solved but i only pass it under unit tests.

I will make actually traffic test tomorrow and update the result. Surely if you still have the codes of scene you can simply retry.

[jasonjoo2010]

I've tested with the code and when I was publishing the rule via the dashboard, an error occurred:
...

The error occurred in the second time of reading:

It seems that the SocketInputStream will wait until timeout, rather than for EOF. Maybe we need to check it via `in.ready()` or other approaches? @jasonjoo2010

Tests have been passed and detail cases are listed below.
And more codes are restructured:

• Reader is replaced with plain InputStream to support Content-Type
• readLine() under InputStream
• StatusCode becomes standalone enum to make response clear

Cases

Cases are run under sentinel-demo-command-handler

Normal Get Request

GET /echo?name=test HTTP/1.1
Host: demo.com
Accept: */*
Accept-Language: en-us
Accept-Encoding: plain/text

=>

HTTP/1.0 200 OK
Content-Length: 11
Connection: close

Hello: test

Normal Post Request

POST /echo?name=fake HTTP/1.1
Host: demo.com
Accept: */*
Accept-Language: en-us
Accept-Encoding: plain/text
Content-Type: application/x-www-form-urlencoded
Content-Length: 9

name=test

=>

HTTP/1.0 200 OK
Content-Length: 11
Connection: close

Hello: test

Smaller Content-Length

POST /echo?name=fake HTTP/1.1
Host: demo.com
Accept: */*
Accept-Language: en-us
Accept-Encoding: plain/text
Content-Type: application/x-www-form-urlencoded
Content-Length: 8

name=test

=>

HTTP/1.0 200 OK
Content-Length: 10
Connection: close

Hello: tes

Incomplete Request

POST /echo?name=fake HTTP/1.1
Host: demo.com
Accept: */*
Accept-Language: en-us
Accept-Encoding: plain/text
Content-Type: application/x-www-form-urlencoded
Content-Length: 100

name=test

That depends. If client close the stream server will just accept and parse it normally but if client flush the buffer and wait without closing stream it will lead to a Read Timeout error and response as HTTP_500.

Malformed Request

POST /echo?name=fake HTTP/1.1
Host: demo.com
Accept: */*
Accept-Language: en-us
Accept-Encoding: plain/text
Content-Type: application/x-www-form-urlencoded
Content-Length: 20

name=test
s
d
o
fihasdoihfoiasdhfiohasdiof

if the body is not encoded correctly server will still accept it(there are illegal newlines in this example)
=>

HTTP/1.0 200 OK
Content-Length: 22
Connection: close

Hello: test
s
d
o
fiha

[sczyh30]

LGTM

[sczyh30]

Nice work. Thanks!