Java: improve models for some important JDK methods - #15280
Conversation
Add a neutral model for it, but also a summary model for `String.valueOf(CharSequence)`
Click to show differences in coveragejavaGenerated file changes for java
- Java Standard Library,``java.*``,10,724,231,79,,9,,,19
+ Java Standard Library,``java.*``,10,733,231,79,,9,,,19
- Totals,,308,18935,2462,331,16,128,33,1,402
+ Totals,,308,18944,2462,331,16,128,33,1,402
- java.lang,33,3,94,,13,,,,,,1,,,,,,,,,,,,8,,,6,,,4,,,1,,,,,,,,,,,,,,3,,,57,37
+ java.lang,33,3,103,,13,,,,,,1,,,,,,,,,,,,8,,,6,,,4,,,1,,,,,,,,,,,,,,3,,,60,43 |
|
Does this PR need a change note? |
Since there's potential for users to get impacted (i.e changes in alerts), I'd say yes. |
atorralba
left a comment
There was a problem hiding this comment.
Models LGTM with 1 minor question.
| - ["java.lang", "Exception", False, "Exception", "(String,Throwable)", "", "Argument[0]", "Argument[this].SyntheticField[java.lang.Throwable.message]", "value", "manual"] | ||
| - ["java.lang", "Exception", False, "Exception", "(String,Throwable)", "", "Argument[1]", "Argument[this].SyntheticField[java.lang.Throwable.cause]", "value", "manual"] | ||
| - ["java.lang", "Exception", False, "Exception", "(Throwable)", "", "Argument[0]", "Argument[this].SyntheticField[java.lang.Throwable.cause]", "value", "manual"] | ||
| - ["java.lang", "Exception", False, "Exception", "(Throwable)", "", "Argument[0].SyntheticField[java.lang.Throwable.message]", "Argument[this].SyntheticField[java.lang.Throwable.message]", "taint", "manual"] |
There was a problem hiding this comment.
Why is this a taint model?
There was a problem hiding this comment.
Good question. It's implemented as a call to the equivalent constructor of the base class Throwable, so I guess I was copying the model for that (also in this PR). And that is implemented using toString(), which prepends the class name in front of the message. So I guess it's the prepending the class name which changes the string.
atorralba
left a comment
There was a problem hiding this comment.
Oh, a test is failing.
|
Hopefully I've fixed CI now. |
While working on dataflow-generated models, I found that these manual models needed to be added/improved to avoid clashes.
Note that I have added a neutral model for
String.valueOf(Object). We had already made a conscious choice not to make a summary model for this method (as noted here), and this neutral model is just recording this fact (and stopping any generated models for it from having an effect). I did add a summary model forString.valueOf(charsequence), and we already have one forString.valueOf(char), which I hope will cover many cases where there should be flow.