[servicecomb-java-chassis] branch master updated: [SCB-2171] avoid leak server ip when process TimeoutException (#2162)

[li...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git


The following commit(s) were added to refs/heads/master by this push:
     new 43975bd  [SCB-2171] avoid leak server ip when process TimeoutException (#2162)
43975bd is described below

commit 43975bd8ca95f76b698e4ba585778c71ff84bb12
Author: wujimin <wu...@huawei.com>
AuthorDate: Thu Dec 24 09:35:44 2020 +0800

    [SCB-2171] avoid leak server ip when process TimeoutException (#2162)
---
 .../converter/TimeoutExceptionConverter.java       | 12 ++++-
 .../converter/TimeoutExceptionConverterTest.java   | 59 ++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/converter/TimeoutExceptionConverter.java b/core/src/main/java/org/apache/servicecomb/core/exception/converter/TimeoutExceptionConverter.java
index cee170d..0059e7f 100644
--- a/core/src/main/java/org/apache/servicecomb/core/exception/converter/TimeoutExceptionConverter.java
+++ b/core/src/main/java/org/apache/servicecomb/core/exception/converter/TimeoutExceptionConverter.java
@@ -26,8 +26,12 @@ import javax.ws.rs.core.Response.StatusType;
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.exception.ExceptionConverter;
 import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class TimeoutExceptionConverter implements ExceptionConverter<TimeoutException> {
+  private static final Logger LOGGER = LoggerFactory.getLogger(TimeoutExceptionConverter.class);
+
   public static final int ORDER = Short.MAX_VALUE;
 
   @Override
@@ -43,9 +47,13 @@ public class TimeoutExceptionConverter implements ExceptionConverter<TimeoutExce
   @Override
   public InvocationException convert(@Nullable Invocation invocation, TimeoutException throwable,
       StatusType genericStatus) {
-    String message = String.format("Request Timeout. Details: %s", throwable.getMessage());
+    // throwable.getMessage:
+    //   The timeout period of 30000ms has been exceeded while executing GET /xxx for server 1.1.1.1:8080
+    // should not copy the message to invocationException to avoid leak server ip address
+    LOGGER.info("Request timeout, Details: {}.", throwable.getMessage());
+
     return new InvocationException(REQUEST_TIMEOUT,
-        ExceptionConverter.getGenericCode(genericStatus), message,
+        ExceptionConverter.getGenericCode(genericStatus), "Request Timeout.",
         throwable);
   }
 }
diff --git a/core/src/test/java/org/apache/servicecomb/core/exception/converter/TimeoutExceptionConverterTest.java b/core/src/test/java/org/apache/servicecomb/core/exception/converter/TimeoutExceptionConverterTest.java
new file mode 100644
index 0000000..6527bd5
--- /dev/null
+++ b/core/src/test/java/org/apache/servicecomb/core/exception/converter/TimeoutExceptionConverterTest.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.servicecomb.core.exception.converter;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.concurrent.TimeoutException;
+
+import javax.ws.rs.core.Response.Status;
+
+import org.apache.servicecomb.core.exception.Exceptions;
+import org.apache.servicecomb.foundation.test.scaffolding.log.LogCollector;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
+import org.junit.jupiter.api.Test;
+
+class TimeoutExceptionConverterTest {
+  private InvocationException convert() {
+    TimeoutException timeoutException = new TimeoutException(
+        "The timeout period of 30000ms has been exceeded while executing GET /xxx for server 1.1.1.1:8080");
+
+    InvocationException exception = Exceptions.convert(null, timeoutException, Status.BAD_REQUEST);
+    return exception;
+  }
+
+  @Test
+  void should_not_leak_server_ip() {
+    InvocationException exception = convert();
+
+    assertThat(exception)
+        .hasMessage(
+            "InvocationException: code=408;msg=CommonExceptionData{code='SCB.00000000', message='Request Timeout.', dynamic={}}");
+  }
+
+  @Test
+  void should_log_detail() {
+    try (LogCollector logCollector = new LogCollector()) {
+      convert();
+
+      assertThat(logCollector.getLastEvents().getRenderedMessage())
+          .isEqualTo(
+              "Request timeout, Details: The timeout period of 30000ms has been exceeded while executing GET /xxx for server 1.1.1.1:8080.");
+    }
+  }
+}
\ No newline at end of file