Repository navigation
Redact HTTP error stack traces by default - #2028
HocVoNgThai wants to merge 2 commits into
Conversation
|
AI Summary This issue addresses the lack of default security redaction by changing the |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughHTTP error responses now redact stack traces by default. Tests cover omitted and explicit property values. The README documents how to include diagnostic traces and states that other response fields remain unchanged. ChangesHTTP Error Stack-Trace Redaction
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Deployments that omit the property will redact stack traces by default; operators can explicitly set it to false to retain diagnostic traces. No merge-blocking issue was identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the error trail Comment |
Fixes #2026.
http.error.excludeStackTracesnow behaves astrue. Set it explicitly tofalseto keep the previous diagnostic stack trace response.What changes did you make?
Change the existing property fallback from
falsetotrue. Keep the existing redaction marker and explicit overrides. Add README migration guidance and four regression cases using the real WebFlux error response pipeline. Document the reviewed constructor, test methods, and fixture helpers with Javadoc; the documentation follow-up changes no runtime behavior.Is there anything you'd like reviewers to focus on?
The default behavior change and preserved opt-in diagnostics. This proposal changes only stack trace inclusion, not exception messages, status codes, logging, or response structure.
How Has This Been Tested?
All four JUnit cases pass using the actual production handler in an offline, resource-limited Java 25 container: omitted property, explicit true, explicit false, and a status-error response. The two default-redaction cases fail on unmodified main and pass after the change.
git diff --checkpasses.The combined focused validation run has 66 passing cases, including the independent permission-validation proposal and documentation checks. Tests use locally available dependency jars rather than a full Gradle build. The full Gradle suite and Checkstyle were not run locally; upstream CI remains required. Existing Lombok/JVM warnings occur on both baseline and patched runs.
Checklist
Focused tests pass; full project CI/Sonar verification remains required. The one-line fallback change needs no inline comment. This PR has no dependency on another proposal.
Summary by CodeRabbit
http.error.excludeStackTracesorHTTP_ERROR_EXCLUDESTACKTRACEStofalseto include them.