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

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


The following commit(s) were added to refs/heads/1.3.x by this push:
     new dc36109  SCB-2108:Fix the problem that SwaggerProducerOperation catchs 
busines… (#2021)
dc36109 is described below

commit dc3610987d5185ef3766631f5629f50d34ca1c38
Author: develpoerX <[email protected]>
AuthorDate: Tue Oct 27 17:05:40 2020 +0800

    SCB-2108:Fix the problem that SwaggerProducerOperation catchs busines… 
(#2021)
    
    * SCB-2108:Fix the problem that SwaggerProducerOperation catchs business 
exceptions and prints logs may reveal sensitive information.
    
    * SCB-2108:Fix the problem that SwaggerProducerOperation catchs busines
---
 .../schema/TestProducerSchemaFactory.java          |  8 ++---
 .../swagger/engine/SwaggerProducerOperation.java   | 37 ++++++++++------------
 2 files changed, 20 insertions(+), 25 deletions(-)

diff --git 
a/core/src/test/java/org/apache/servicecomb/core/definition/schema/TestProducerSchemaFactory.java
 
b/core/src/test/java/org/apache/servicecomb/core/definition/schema/TestProducerSchemaFactory.java
index f33b47d..d9c7a82 100644
--- 
a/core/src/test/java/org/apache/servicecomb/core/definition/schema/TestProducerSchemaFactory.java
+++ 
b/core/src/test/java/org/apache/servicecomb/core/definition/schema/TestProducerSchemaFactory.java
@@ -170,8 +170,8 @@ public class TestProducerSchemaFactory {
     Assert.assertEquals(true, holder.value.isFailed());
     exception = holder.value.getResult();
     data = (CommonExceptionData) exception.getErrorData();
-    Assert.assertEquals(Status.BAD_REQUEST.getStatusCode(), 
exception.getStatusCode());
-    Assert.assertEquals("Parameters not valid or types not match.", 
data.getMessage());
+    Assert.assertEquals(590, exception.getStatusCode());
+    Assert.assertEquals("Cse Internal Server Error", data.getMessage());
     Assert.assertEquals(nanoTime, 
invocation.getInvocationStageTrace().getStartBusinessMethod());
     Assert.assertEquals(nanoTime, 
invocation.getInvocationStageTrace().getFinishBusiness());
   }
@@ -235,8 +235,8 @@ public class TestProducerSchemaFactory {
     Assert.assertEquals(true, holder.value.isFailed());
     exception = holder.value.getResult();
     data = (CommonExceptionData) exception.getErrorData();
-    Assert.assertEquals(Status.BAD_REQUEST.getStatusCode(), 
exception.getStatusCode());
-    Assert.assertEquals("Parameters not valid or types not match.", 
data.getMessage());
+    Assert.assertEquals(590, exception.getStatusCode());
+    Assert.assertEquals("Cse Internal Server Error", data.getMessage());
     Assert.assertEquals(nanoTime, 
invocation.getInvocationStageTrace().getStartBusinessMethod());
     Assert.assertEquals(nanoTime, 
invocation.getInvocationStageTrace().getFinishBusiness());
   }
diff --git 
a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerProducerOperation.java
 
b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerProducerOperation.java
index 5619c15..6392bce 100644
--- 
a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerProducerOperation.java
+++ 
b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerProducerOperation.java
@@ -151,17 +151,11 @@ public class SwaggerProducerOperation {
 
         asyncResp.handle(processException(invocation, ex));
       });
-    } catch (IllegalArgumentException ae) {
-      LOGGER.error("Parameters not valid or types not match {},",
-          invocation.getInvocationQualifiedName(), ae);
-      invocation.onBusinessMethodFinish();
-      invocation.onBusinessFinish();
-      asyncResp.handle(processException(invocation,
-          new InvocationException(Status.BAD_REQUEST.getStatusCode(), "",
-              new CommonExceptionData("Parameters not valid or types not 
match."), ae)));
     } catch (Throwable e) {
-      LOGGER.error("unexpected error {},",
-          invocation.getInvocationQualifiedName(), e);
+      if (shouldPrintErrorLog(e)){
+        LOGGER.error("unexpected error operation={}, message={}",
+            invocation.getInvocationQualifiedName(), e.getMessage());
+      }
       invocation.onBusinessMethodFinish();
       invocation.onBusinessFinish();
       asyncResp.handle(processException(invocation, e));
@@ -190,18 +184,11 @@ public class SwaggerProducerOperation {
 
       invocation.onBusinessMethodFinish();
       invocation.onBusinessFinish();
-    } catch (IllegalArgumentException ae) {
-      LOGGER.error("Parameters not valid or types not match {},",
-          invocation.getInvocationQualifiedName(), ae);
-      invocation.onBusinessMethodFinish();
-      invocation.onBusinessFinish();
-      // ae.getMessage() is always null. Give a custom error message.
-      response = processException(invocation,
-          new InvocationException(Status.BAD_REQUEST.getStatusCode(), "",
-              new CommonExceptionData("Parameters not valid or types not 
match."), ae));
     } catch (Throwable e) {
-      LOGGER.error("unexpected error {},",
-          invocation.getInvocationQualifiedName(), e);
+      if (shouldPrintErrorLog(e)){
+        LOGGER.error("unexpected error operation={}, message={}",
+            invocation.getInvocationQualifiedName(), e.getMessage());
+      }
       invocation.onBusinessMethodFinish();
       invocation.onBusinessFinish();
       response = processException(invocation, e);
@@ -209,6 +196,14 @@ public class SwaggerProducerOperation {
     return response;
   }
 
+  protected boolean shouldPrintErrorLog(Throwable throwable) {
+    if (!(throwable instanceof InvocationTargetException)) {
+      return true;
+    }
+    Throwable targetException = ((InvocationTargetException) 
throwable).getTargetException();
+    return !(targetException instanceof InvocationException);
+  }
+
   protected Response processException(SwaggerInvocation invocation, Throwable 
e) {
     if (InvocationTargetException.class.isInstance(e)) {
       e = ((InvocationTargetException) e).getTargetException();

Reply via email to