Skip to content

[SCB-2387] optimize abnormal print information of SwaggerProducerOper… #2722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
david6969xin wants to merge 2 commits into from
Closed

[SCB-2387] optimize abnormal print information of SwaggerProducerOper… #2722

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6b495c
[SCB-2387] optimize abnormal print information of SwaggerProducerOper…
david6969xin Feb 18, 2022
8f5a659
[SCB-2387] optimize abnormal print information of SwaggerProducerOper…
david6969xin Feb 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions ...on-core/src/main/java/org/apache/servicecomb/swagger/engine/SwaggerProducerOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.concurrent.CompletableFuture;


import org.apache.servicecomb.foundation.common.utils.ExceptionUtils;
import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
import org.apache.servicecomb.swagger.invocation.AsyncResponse;
import org.apache.servicecomb.swagger.invocation.Response;
Expand Down Expand Up @@ -153,7 +152,7 @@ public void doCompletableFutureInvoke(SwaggerInvocation invocation, AsyncRespons
} catch (Throwable e) {
if (shouldPrintErrorLog(e)) {
LOGGER.error("unexpected error operation={}, message={}",
invocation.getInvocationQualifiedName(), ExceptionUtils.getExceptionMessageWithoutTrace(e));
invocation.getInvocationQualifiedName(), e.getMessage());
Copy link
Copy Markdown
Contributor

@liubao68 liubao68 Feb 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why modify this ? There is no SCB-2387 for this.

Copy link
Copy Markdown
Contributor Author

@david6969xin david6969xin Feb 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code will print too much sensitive information when JSON parsing fails

Copy link
Copy Markdown
Member

@yhs0092 yhs0092 Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all of the sensitive contents can be blocked by this way. For example, the Jackson may put raw JSON string into exception message.

Copy link
Copy Markdown
Contributor

@liubao68 liubao68 Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I'd prefer not modify this. This is not an actual serious information leak problem. And if the user's do need fix it, they can add try-catch block to avoid throw JsonParseException.

This information is quite useful for problem analysis and have modified many times.

}
invocation.onBusinessMethodFinish();
invocation.onBusinessFinish();
Expand Down Expand Up @@ -186,7 +185,7 @@ public Response doInvoke(SwaggerInvocation invocation) {
} catch (Throwable e) {
if (shouldPrintErrorLog(e)) {
LOGGER.error("unexpected error operation={}, message={}",
invocation.getInvocationQualifiedName(), ExceptionUtils.getExceptionMessageWithoutTrace(e));
invocation.getInvocationQualifiedName(), e.getMessage());
}
invocation.onBusinessMethodFinish();
invocation.onBusinessFinish();
Expand Down