Skip to content
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.

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

BUG 35047999: Show Server Error in faultString instead of error details. #648

Merged
merged 8 commits into from Jun 23, 2023

Conversation

himanshuatgit
Copy link
Contributor

As of now, SOAP response packet contains faultString which contains detailed error message in case of any SOAP Fault.

The customer has a requirement where they do not want any exception messages to be displayed in the faultString for security concerns.

For the same reason, a fix was introduced for BUG34600318 where a new system property com.sun.xml.ws.fault.SOAPFaultBuilder.captureExceptionMessage=false
has been added to simulate customer intended behavior.

This bug is intended to address the same issue but to include all possible cases which has been missed in the previous BUG34600318.

Added more testcases in file:
./jaxws-ri/runtime/rt/src/test/java/com/sun/xml/ws/fault/SOAPFaultBuilderTest.java

Signed-off-by: Himanshu Ranjan himanshu.ranjan@oracle.com

Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
@himanshuatgit himanshuatgit requested a review from lukasj June 1, 2023 10:46
Copy link
Member

@lukasj lukasj left a comment

Choose a reason for hiding this comment

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

  • new private method createFaultString(String)::String (or similar) responsible for altering existing fault string if necessary should be added to SOAPFaultBuilder and existing createSOAP1?Fault methods should be updated to send the fault string through this new one

  • the pattern to test statics/system props in this case should be:

boolean oldValue = SOAPFaultBuilder.isCaptureExceptionMessage();
try {
    SOAPFaultBuilder.setCaptureExceptionMessage(...);
} finally {
    SOAPFaultBuilder.setCaptureExceptionMessage(oldValue);
}

Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
@himanshuatgit
Copy link
Contributor Author

Changes done. Kindly review.

@@ -501,6 +506,9 @@ private static Message createSOAP12Fault(SOAPVersion soapVersion, Throwable e, O
}
}

faultString = setFaultStringToServerError(faultString);
Copy link
Member

Choose a reason for hiding this comment

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

changing passed in argument is not good idea, even though java is pass-by-value language. Should use a different variable if at all required (...likely not in this case)

Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
@himanshuatgit
Copy link
Contributor Author

Changes done. Please review.

Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
@himanshuatgit
Copy link
Contributor Author

Pull request updated. Please have a look.

Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
Copy link
Member

@lukasj lukasj left a comment

Choose a reason for hiding this comment

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

LGTM

@lukasj lukasj merged commit 9203195 into eclipse-ee4j:master Jun 23, 2023
5 checks passed
lukasj pushed a commit to lukasj/metro-jax-ws that referenced this pull request Oct 17, 2023
…ls. (eclipse-ee4j#648)

Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
(cherry picked from commit 9203195)
lukasj pushed a commit that referenced this pull request Oct 17, 2023
…ls. (#648)

Signed-off-by: Himanshu Ranjan <himanshu.ranjan@oracle.com>
(cherry picked from commit 9203195)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants