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

Throwable.getMessage is unsafe by default #2151

Merged
merged 4 commits into from
Mar 31, 2022

Conversation

carterkozak
Copy link
Contributor

==COMMIT_MSG==
Throwable.getMessage is unsafe by default
==COMMIT_MSG==

@changelog-app
Copy link

changelog-app bot commented Mar 30, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Throwable.getMessage is unsafe by default

Check the box to generate changelog(s)

  • Generate changelog entry

@policy-bot policy-bot bot requested a review from jkozlowski March 30, 2022 19:04
@@ -137,6 +137,11 @@
private static final Matcher<ExpressionTree> TO_STRING =
MethodMatchers.instanceMethod().anyClass().named("toString").withNoParameters();

private static final Matcher<ExpressionTree> THROWABLE_GET_MESSAGE = MethodMatchers.instanceMethod()
.onDescendantOf(Throwable.class.getName())
.named("getMessage")
Copy link
Contributor

Choose a reason for hiding this comment

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

we should also treat Throwable#getLocalizedMessage() as unsafe as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea, updated! Let me know if you think of other types/methods which are always unsafe.

Throwable.toString() is unsafe as well, but I'd like to handle that differently later on in a way that captures string covnersion e.g. "exception: " + myException;

Copy link
Contributor

@fawind fawind left a comment

Choose a reason for hiding this comment

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

Looks good! Can follow up if we think of more types/methods that are always unsafe.

@bulldozer-bot bulldozer-bot bot merged commit eb638bb into develop Mar 31, 2022
@bulldozer-bot bulldozer-bot bot deleted the ckozak/throwabe_message_unsafe branch March 31, 2022 12:00
@svc-autorelease
Copy link
Collaborator

Released 4.85.0

bulldozer-bot bot pushed a commit to palantir/witchcraft-api that referenced this pull request Mar 31, 2022
###### _excavator_ is a bot for automating changes across repositories.

Changes produced by the roomba/latest-baseline-oss check.

# Release Notes
## 4.85.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | Throwable.getMessage is unsafe by default | palantir/gradle-baseline#2151 |


## 4.86.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | Ban java deserialization | palantir/gradle-baseline#2152 |


## 4.87.0
| Type | Description | Link |
| ---- | ----------- | ---- |
| Improvement | Array assignment merges safety rather than replacing it | palantir/gradle-baseline#2154 |



To enable or disable this check, please contact the maintainers of Excavator.
This was referenced Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants