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();