This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch 2.8.x
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/2.8.x by this push:
new d38b9e970 [SCB-2864]upgrade vertx and fix connection pool problem and
enable log activity (#4262)
d38b9e970 is described below
commit d38b9e970f9e91541963cab62e422e6b86118e0a
Author: liubao68 <[email protected]>
AuthorDate: Thu Mar 7 13:32:55 2024 +0800
[SCB-2864]upgrade vertx and fix connection pool problem and enable log
activity (#4262)
---
.../core/governance/GovernanceConfiguration.java | 4 +-
.../springmvc/client/TestMaxHttpUrlLength.java | 11 +-
dependencies/default/pom.xml | 4 +-
.../vertx/client/http/HttpClientOptionsSPI.java | 3 +
pom.xml | 4 -
.../client/http/RegistryHttpClientOptionsSPI.java | 5 +
.../client/HttpTransportHttpClientOptionsSPI.java | 5 +
.../rest/client/TransportClientConfig.java | 6 ++
.../transport/rest/vertx/RestBodyHandler.java | 117 ++++++++++++---------
.../rest/vertx/MockHttpServerResponse.java | 6 ++
10 files changed, 105 insertions(+), 60 deletions(-)
diff --git
a/core/src/main/java/org/apache/servicecomb/core/governance/GovernanceConfiguration.java
b/core/src/main/java/org/apache/servicecomb/core/governance/GovernanceConfiguration.java
index 84a869815..33f86e6c7 100644
---
a/core/src/main/java/org/apache/servicecomb/core/governance/GovernanceConfiguration.java
+++
b/core/src/main/java/org/apache/servicecomb/core/governance/GovernanceConfiguration.java
@@ -46,8 +46,8 @@ public class GovernanceConfiguration {
}
public static long getWithDuration(String microservice) {
- final long defaultValue = 0;
- String duration = getStringProperty("0", ROOT + microservice + "." +
WITH_DURATION,
+ final long defaultValue = 1;
+ String duration = getStringProperty("1", ROOT + microservice + "." +
WITH_DURATION,
ROOT + WITH_DURATION);
try {
long result = Long.parseLong(duration);
diff --git
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestMaxHttpUrlLength.java
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestMaxHttpUrlLength.java
index 56b64c734..2cbc09a7d 100644
---
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestMaxHttpUrlLength.java
+++
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestMaxHttpUrlLength.java
@@ -22,7 +22,7 @@ import org.apache.servicecomb.demo.TestMgr;
import org.apache.servicecomb.provider.springmvc.reference.RestTemplateBuilder;
import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.springframework.stereotype.Component;
-import org.springframework.web.client.RestTemplate;
+import org.springframework.web.client.RestOperations;
import com.google.common.base.Strings;
import com.netflix.config.DynamicPropertyFactory;
@@ -37,14 +37,15 @@ public class TestMaxHttpUrlLength implements
CategorizedTestCase {
}
private void testUrlNotLongerThan4096() {
- RestTemplate restTemplate = RestTemplateBuilder.create();
-
- String q = Strings.repeat("q", 4096 - "GET
/api/springmvc/controller/sayhi?name=".length() - " HTTP/1.1\r".length());
+ RestOperations restTemplate = RestTemplateBuilder.create();
+ // \r doesn't count for url length Since netty 4.1.88.Final See
https://github.com/netty/netty/pull/12321
+ String q = Strings.repeat("q",
+ 4096 + 1 - "GET /api/springmvc/controller/sayhi?name=".length() - "
HTTP/1.1\r".length());
TestMgr.check("hi " + q + " [" + q + "]",
restTemplate.getForObject("cse://springmvc/springmvc/controller/sayhi?name=" +
q,
String.class));
- q = Strings.repeat("q", 4096 + 1 - "GET
/api/springmvc/controller/sayhi?name=".length() - " HTTP/1.1\r".length());
+ q = Strings.repeat("q", 4096 + 2 - "GET
/api/springmvc/controller/sayhi?name=".length() - " HTTP/1.1\r".length());
try {
restTemplate.getForObject("cse://springmvc/springmvc/controller/sayhi?name=" +
q,
String.class);
diff --git a/dependencies/default/pom.xml b/dependencies/default/pom.xml
index 7a16e1e2c..e3a86de7f 100644
--- a/dependencies/default/pom.xml
+++ b/dependencies/default/pom.xml
@@ -78,7 +78,7 @@
<mock-server.version>5.14.0</mock-server.version>
<nacos-client.version>2.2.0</nacos-client.version>
<netflix-commons.version>0.3.0</netflix-commons.version>
- <netty.version>4.1.87.Final</netty.version>
+ <netty.version>4.1.106.Final</netty.version>
<okhttp3.version>4.10.0</okhttp3.version>
<prometheus.version>0.16.0</prometheus.version>
<protobuf.version>3.21.12</protobuf.version>
@@ -98,7 +98,7 @@
<spring-boot.version>2.7.15</spring-boot.version>
<swagger.version>1.6.9</swagger.version>
<swagger2markup.version>1.3.3</swagger2markup.version>
- <vertx.version>4.4.4</vertx.version>
+ <vertx.version>4.4.8</vertx.version>
<zipkin.version>2.24.0</zipkin.version>
<zipkin-reporter.version>2.16.3</zipkin-reporter.version>
<!-- Base dir of main -->
diff --git
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/http/HttpClientOptionsSPI.java
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/http/HttpClientOptionsSPI.java
index 92a84e67f..14b5cbed1 100644
---
a/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/http/HttpClientOptionsSPI.java
+++
b/foundations/foundation-vertx/src/main/java/org/apache/servicecomb/foundation/vertx/client/http/HttpClientOptionsSPI.java
@@ -79,6 +79,8 @@ public interface HttpClientOptionsSPI {
int getKeepAliveTimeout();
+ boolean enableLogActivity();
+
/***************** http 2 settings ****************************/
int getHttp2MultiplexingLimit();
@@ -113,6 +115,7 @@ public interface HttpClientOptionsSPI {
httpClientOptions.setMaxPoolSize(spi.getMaxPoolSize());
httpClientOptions.setKeepAlive(spi.isKeepAlive());
httpClientOptions.setMaxHeaderSize(spi.getMaxHeaderSize());
+ httpClientOptions.setLogActivity(spi.enableLogActivity());
if (spi.isProxyEnable()) {
ProxyOptions proxy = new ProxyOptions();
diff --git a/pom.xml b/pom.xml
index e6f3a22f6..941b88ff9 100644
--- a/pom.xml
+++ b/pom.xml
@@ -41,7 +41,6 @@
<java.version>1.8</java.version>
<argLine>-Dfile.encoding=UTF-8</argLine>
<skip-remote-resource>true</skip-remote-resource>
- <werror.properties>-Werror</werror.properties>
<test.additional.args/>
<!-- plugin version start -->
<!-- sort by alpha -->
@@ -407,7 +406,6 @@
<showDeprecation>true</showDeprecation>
<showWarnings>true</showWarnings>
<compilerArgs>
- <arg>${werror.properties}</arg>
<arg>-Xlint:all</arg>
<!--not care for jdk8/jdk7 compatible problem-->
<arg>-Xlint:-classfile</arg>
@@ -656,7 +654,6 @@
<jdk>[11,17)</jdk>
</activation>
<properties>
- <werror.properties/>
<test.additional.args>
-Djdk.attach.allowAttachSelf
--add-opens java.base/java.lang=ALL-UNNAMED
@@ -670,7 +667,6 @@
<jdk>[17,)</jdk>
</activation>
<properties>
- <werror.properties/>
<test.additional.args>
-Djdk.attach.allowAttachSelf
--add-opens java.base/java.io=ALL-UNNAMED
diff --git
a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/http/RegistryHttpClientOptionsSPI.java
b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/http/RegistryHttpClientOptionsSPI.java
index d7b5c3cac..6f55120ff 100644
---
a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/http/RegistryHttpClientOptionsSPI.java
+++
b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/client/http/RegistryHttpClientOptionsSPI.java
@@ -137,6 +137,11 @@ public class RegistryHttpClientOptionsSPI implements
HttpClientOptionsSPI {
return result;
}
+ @Override
+ public boolean enableLogActivity() {
+ return false;
+ }
+
@Override
public int getHttp2MultiplexingLimit() {
return HttpClientOptions.DEFAULT_HTTP2_MULTIPLEXING_LIMIT;
diff --git
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/HttpTransportHttpClientOptionsSPI.java
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/HttpTransportHttpClientOptionsSPI.java
index ab7a96309..369443c14 100644
---
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/HttpTransportHttpClientOptionsSPI.java
+++
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/HttpTransportHttpClientOptionsSPI.java
@@ -131,6 +131,11 @@ public class HttpTransportHttpClientOptionsSPI implements
HttpClientOptionsSPI {
return TransportClientConfig.getConnectionKeepAliveTimeoutInSeconds();
}
+ @Override
+ public boolean enableLogActivity() {
+ return TransportClientConfig.enableLogActivity();
+ }
+
@Override
public int getHttp2MultiplexingLimit() {
return HttpClientOptions.DEFAULT_HTTP2_MULTIPLEXING_LIMIT;
diff --git
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
index d0be3b82c..8e0ac0d8c 100644
---
a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
+++
b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/TransportClientConfig.java
@@ -103,6 +103,12 @@ public final class TransportClientConfig {
.get();
}
+ public static boolean enableLogActivity() {
+ return DynamicPropertyFactory.getInstance()
+ .getBooleanProperty("servicecomb.rest.client.enableLogActivity", false)
+ .get();
+ }
+
public static int getHttp2ConnectionKeepAliveTimeoutInSeconds() {
return DynamicPropertyFactory.getInstance()
.getIntProperty("servicecomb.rest.client.http2.connection.keepAliveTimeoutInSeconds",
diff --git
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestBodyHandler.java
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestBodyHandler.java
index f6e02fe02..54e1331a8 100644
---
a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestBodyHandler.java
+++
b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestBodyHandler.java
@@ -24,14 +24,12 @@ package org.apache.servicecomb.transport.rest.vertx;
import java.io.File;
import java.util.List;
import java.util.UUID;
-import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
-import com.google.common.annotations.VisibleForTesting;
import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
import org.apache.servicecomb.swagger.invocation.exception.ExceptionFactory;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.VisibleForTesting;
import io.netty.handler.codec.DecoderException;
import io.netty.handler.codec.http.HttpHeaderValues;
@@ -41,10 +39,11 @@ import io.vertx.core.buffer.Buffer;
import io.vertx.core.file.FileSystem;
import io.vertx.core.http.HttpHeaders;
import io.vertx.core.http.HttpServerRequest;
+import io.vertx.core.http.HttpServerResponse;
+import io.vertx.core.http.HttpVersion;
import io.vertx.ext.web.FileUpload;
import io.vertx.ext.web.RoutingContext;
import io.vertx.ext.web.handler.BodyHandler;
-import io.vertx.ext.web.handler.impl.BodyHandlerImpl;
import io.vertx.ext.web.impl.FileUploadImpl;
import io.vertx.ext.web.impl.RoutingContextInternal;
@@ -56,8 +55,6 @@ import io.vertx.ext.web.impl.RoutingContextInternal;
*/
public class RestBodyHandler implements BodyHandler {
- private static final Logger LOGGER =
LoggerFactory.getLogger(BodyHandlerImpl.class);
-
private long bodyLimit = DEFAULT_BODY_LIMIT;
private boolean handleFileUploads;
@@ -98,7 +95,9 @@ public class RestBodyHandler implements BodyHandler {
@Override
public void handle(RoutingContext context) {
- HttpServerRequest request = context.request();
+ final HttpServerRequest request = context.request();
+ final HttpServerResponse response = context.response();
+
if (request.headers().contains(HttpHeaders.UPGRADE, HttpHeaders.WEBSOCKET,
true)) {
context.next();
return;
@@ -112,11 +111,59 @@ public class RestBodyHandler implements BodyHandler {
// we need to keep state since we can be called again on reroute
if (!((RoutingContextInternal)
context).seenHandler(RoutingContextInternal.BODY_HANDLER)) {
- long contentLength = isPreallocateBodyBuffer ?
parseContentLengthHeader(request) : -1;
- BHandler handler = new BHandler(context, contentLength);
- request.handler(handler);
- request.endHandler(v -> handler.end());
((RoutingContextInternal)
context).visitHandler(RoutingContextInternal.BODY_HANDLER);
+
+ // Check if a request has a request body.
+ // A request with a body __must__ either have `transfer-encoding`
+ // or `content-length` headers set.
+ // http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.3
+ final long parsedContentLength = parseContentLengthHeader(request);
+ // http2 never transmits a `transfer-encoding` as frames are chunks.
+ final boolean hasTransferEncoding =
+ request.version() == HttpVersion.HTTP_2 ||
request.headers().contains(HttpHeaders.TRANSFER_ENCODING);
+
+ if (!hasTransferEncoding && parsedContentLength == -1) {
+ // there is no "body", so we can skip this handler
+ context.next();
+ return;
+ }
+
+ // before parsing the body we can already discard a bad request just by
inspecting the content-length against
+ // the body limit, this will reduce load, on the server by totally
skipping parsing the request body
+ if (bodyLimit != -1 && parsedContentLength != -1) {
+ if (parsedContentLength > bodyLimit) {
+ context.fail(413);
+ return;
+ }
+ }
+
+ // handle expectations
+ // https://httpwg.org/specs/rfc7231.html#header.expect
+ final String expect = request.getHeader(HttpHeaders.EXPECT);
+ if (expect != null) {
+ // requirements validation
+ if (expect.equalsIgnoreCase("100-continue")) {
+ // A server that receives a 100-continue expectation in an HTTP/1.0
request MUST ignore that expectation.
+ if (request.version() != HttpVersion.HTTP_1_0) {
+ // signal the client to continue
+ response.writeContinue();
+ }
+ } else {
+ // the server cannot meet the expectation, we only know about
100-continue
+ context.fail(417);
+ return;
+ }
+ }
+
+ final BHandler handler = new BHandler(context, isPreallocateBodyBuffer ?
parsedContentLength : -1);
+ boolean ended = request.isEnded();
+ if (!ended) {
+ request
+ // resume the request (if paused)
+ .handler(handler)
+ .endHandler(handler::end)
+ .resume();
+ }
} else {
// on reroute we need to re-merge the form params if that was desired
if (mergeFormAttributes && request.isExpectMultipart()) {
@@ -187,9 +234,7 @@ public class RestBodyHandler implements BodyHandler {
boolean failed;
- AtomicInteger uploadCount = new AtomicInteger();
-
- AtomicBoolean cleanup = new AtomicBoolean(false);
+ final AtomicInteger uploadCount = new AtomicInteger();
boolean ended;
@@ -240,7 +285,7 @@ public class RestBodyHandler implements BodyHandler {
long size = uploadSize + upload.size();
if (size > bodyLimit) {
failed = true;
- cancelAndCleanupFileUploads();
+ context.cancelAndCleanupFileUploads();
context.fail(413);
return;
}
@@ -249,14 +294,14 @@ public class RestBodyHandler implements BodyHandler {
// we actually upload to a file with a generated filename
uploadCount.incrementAndGet();
String uploadedFileName = new File(uploadsDir,
UUID.randomUUID().toString()).getPath();
- FileUploadImpl fileUpload = new FileUploadImpl(uploadedFileName,
upload);
+ FileUploadImpl fileUpload = new
FileUploadImpl(context.vertx().fileSystem(), uploadedFileName, upload);
fileUploads.add(fileUpload);
Future<Void> fut = upload.streamToFileSystem(uploadedFileName);
fut.onComplete(ar -> {
if (fut.succeeded()) {
uploadEnded();
} else {
- cancelAndCleanupFileUploads();
+ context.cancelAndCleanupFileUploads();
context.fail(ar.cause());
}
});
@@ -265,7 +310,7 @@ public class RestBodyHandler implements BodyHandler {
}
context.request().exceptionHandler(t -> {
- cancelAndCleanupFileUploads();
+ context.cancelAndCleanupFileUploads();
if (t instanceof DecoderException) {
// bad request
context.fail(400, t.getCause());
@@ -312,7 +357,7 @@ public class RestBodyHandler implements BodyHandler {
uploadSize += buff.length();
if (bodyLimit != -1 && uploadSize > bodyLimit) {
failed = true;
- cancelAndCleanupFileUploads();
+ context.cancelAndCleanupFileUploads();
context.fail(413);
} else {
// multipart requests will not end up in the request body
@@ -335,7 +380,7 @@ public class RestBodyHandler implements BodyHandler {
}
}
- void end() {
+ void end(Void v) {
// this marks the end of body parsing, calling doEnd should
// only be possible from this moment onwards
ended = true;
@@ -348,46 +393,24 @@ public class RestBodyHandler implements BodyHandler {
void doEnd() {
- if (failed) {
- cancelAndCleanupFileUploads();
+ if (failed || context.failed()) {
+ context.cancelAndCleanupFileUploads();
return;
}
if (deleteUploadedFilesOnEnd) {
- context.addBodyEndHandler(x -> cancelAndCleanupFileUploads());
+ context.addBodyEndHandler(x -> context.cancelAndCleanupFileUploads());
}
HttpServerRequest req = context.request();
if (mergeFormAttributes && req.isExpectMultipart()) {
req.params().addAll(req.formAttributes());
}
- if (context instanceof RoutingContextInternal) {
- RoutingContextInternal contextInternal = (RoutingContextInternal)
context;
- contextInternal.setBody(body);
- }
+ ((RoutingContextInternal) context).setBody(body);
// release body as it may take lots of memory
body = null;
context.next();
}
-
- /**
- * Cancel all unfinished file upload in progress and delete all uploaded
files.
- */
- private void cancelAndCleanupFileUploads() {
- if (cleanup.compareAndSet(false, true) && handleFileUploads) {
- for (FileUpload fileUpload : context.fileUploads()) {
- FileSystem fileSystem = context.vertx().fileSystem();
- if (!fileUpload.cancel()) {
- String uploadedFileName = fileUpload.uploadedFileName();
- fileSystem.delete(uploadedFileName, deleteResult -> {
- if (deleteResult.failed()) {
- LOGGER.warn("Delete of uploaded file failed: " +
uploadedFileName, deleteResult.cause());
- }
- });
- }
- }
- }
- }
}
}
diff --git
a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/MockHttpServerResponse.java
b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/MockHttpServerResponse.java
index 1c42f093b..568aab893 100644
---
a/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/MockHttpServerResponse.java
+++
b/transports/transport-rest/transport-rest-vertx/src/test/java/org/apache/servicecomb/transport/rest/vertx/MockHttpServerResponse.java
@@ -26,6 +26,7 @@ import io.vertx.core.buffer.Buffer;
import io.vertx.core.http.Cookie;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.http.HttpServerResponse;
+import io.vertx.core.net.HostAndPort;
import java.util.HashMap;
import java.util.Map;
@@ -316,6 +317,11 @@ class MockHttpServerResponse implements HttpServerResponse
{
return null;
}
+ @Override
+ public Future<HttpServerResponse> push(HttpMethod httpMethod, HostAndPort
hostAndPort, String s, MultiMap multiMap) {
+ return null;
+ }
+
@Override
public Future<HttpServerResponse> push(HttpMethod method, String host,
String path, MultiMap headers) {
return Future.succeededFuture();
