Java: Update models for commons-io and add negative models. #10170

michaelnebel · 2022-08-25T10:11:42Z

This this PR

Update existing models for commons-io.
Generate Negative models for commons-io.
The non-supported external api telemetry query no will no longer report any APIs with negative summaries.
Models are not produced for static initializers.

The output was produced by:

Delete the models content from IOGenerated.qll
Run our generation script
time python3 GenerateFlowModel.py --with-sinks --with-summaries --with-negative-summaries ~/Work/databases/java/commons-io "apache/IOGenerated.qll"

github-actions · 2022-08-25T10:14:53Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,561,104,89,,,,,,15
+    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,556,106,91,,,,,,15
-    Totals,,217,6438,1474,117,6,10,107,33,1,84
+    Totals,,217,6433,1476,119,6,10,107,33,1,84

Changes to framework-coverage-java.csv:

- org.apache.commons.io,104,,561,,89,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,547,14
+ org.apache.commons.io,106,,556,,91,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,542,14

github-actions · 2022-08-26T11:50:50Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,561,104,89,,,,,,15
+    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,556,106,91,,,,,,15
-    Totals,,217,6438,1474,117,6,10,107,33,1,84
+    Totals,,217,6433,1476,119,6,10,107,33,1,84

Changes to framework-coverage-java.csv:

- org.apache.commons.io,104,,561,,89,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,547,14
+ org.apache.commons.io,106,,556,,91,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,542,14

github-actions · 2022-08-29T08:43:20Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,561,104,89,,,,,,15
+    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,556,106,91,,,,,,15
-    Totals,,217,6438,1474,117,6,10,107,33,1,84
+    Totals,,217,6433,1476,119,6,10,107,33,1,84

Changes to framework-coverage-java.csv:

- org.apache.commons.io,104,,561,,89,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,547,14
+ org.apache.commons.io,106,,556,,91,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,542,14

github-actions · 2022-08-29T09:07:02Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,561,104,89,,,,,,15
+    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,556,106,91,,,,,,15
-    Totals,,217,6438,1474,117,6,10,107,33,1,84
+    Totals,,217,6433,1476,119,6,10,107,33,1,84

Changes to framework-coverage-java.csv:

- org.apache.commons.io,104,,561,,89,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,547,14
+ org.apache.commons.io,106,,556,,91,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,542,14

github-actions · 2022-08-29T09:56:48Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,561,104,89,,,,,,15
+    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,556,106,91,,,,,,15
-    Totals,,217,6438,1474,117,6,10,107,33,1,84
+    Totals,,217,6433,1476,119,6,10,107,33,1,84

Changes to framework-coverage-java.csv:

- org.apache.commons.io,104,,561,,89,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,547,14
+ org.apache.commons.io,106,,556,,91,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,542,14

…ry query.

…cluded.

github-actions · 2022-08-29T12:34:45Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,561,104,89,,,,,,15
+    `Apache Commons IO <https://commons.apache.org/proper/commons-io/>`_,``org.apache.commons.io``,,556,106,91,,,,,,15
-    Totals,,217,6438,1474,117,6,10,107,33,1,84
+    Totals,,217,6433,1476,119,6,10,107,33,1,84

Changes to framework-coverage-java.csv:

- org.apache.commons.io,104,,561,,89,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,547,14
+ org.apache.commons.io,106,,556,,91,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,542,14

tamasvajk

Looks plausible to me. It would be great to understand why there are changes in the generated models.

tamasvajk · 2022-08-30T14:29:55Z

java/ql/src/utils/model-generator/internal/CaptureModelsSpecific.qll

@@ -55,7 +55,8 @@ private predicate isJdkInternal(J::CompilationUnit cu) {
 private predicate isRelevantForModels(J::Callable api) {
  not isInTestFile(api.getCompilationUnit().getFile()) and
  not isJdkInternal(api.getCompilationUnit()) and
-  not api instanceof J::MainMethod
+  not api instanceof J::MainMethod and
+  not api instanceof J::StaticInitializer


Why are static initializers not relevant for model generation?

They showed up in the negative generated summary models and I don't think it makes sense to include them for positive models either as the static initializers are not explicitly called.

michaelnebel · 2022-08-31T08:42:46Z

Looks plausible to me. It would be great to understand why there are changes in the generated models.

Yes, but I think that needs to be a separate project where we need to improve the tooling such that we can "see" what changes causes the models to change.

intrigus-lgtm · 2022-08-31T12:17:16Z

java/ql/lib/semmle/code/java/frameworks/apache/NegativeIOGenerated.qll

+        "org.apache.commons.io.output;BrokenWriter;BrokenWriter;(IOException);generated",
+        "org.apache.commons.io.output;ByteArrayOutputStream;ByteArrayOutputStream;();generated",
+        "org.apache.commons.io.output;ByteArrayOutputStream;ByteArrayOutputStream;(int);generated",
+        "org.apache.commons.io.output;ByteArrayOutputStream;toBufferedInputStream;(InputStream);generated",


It may be that I'm misunderstanding negative models, but why should there be no flow here?

public static InputStream toBufferedInputStream(InputStream input)
throws IOException

Fetches entire contents of an InputStream and represent same data as result InputStream.

Yes, that would seem reasonable. The negative models are generated in the following way:
If the model generator does not discover flow, then we generate a negative model (stating there is no flow). The model generator is far from perfect and this is probably a case, where "positive" flow is missing.
In any case, the negative models are only used for telemetry purposes at the moment, so they don't interfere with dataflow analysis as such.

I didn't know that they are only for telemetry.
Thank you for the explanation.

github-actions bot added the Java label Aug 25, 2022

github-actions bot added the C# label Aug 26, 2022

michaelnebel added 8 commits August 29, 2022 14:28

Java: Update commons-io generated positive models based on main.

beb85c2

Java: Add negative models for commons-io.

23e0ee6

Java: Use negative summary models in unsupported external api telemet…

290c35e

…ry query.

Java: Add negative model CSV validation test.

dbfd166

C#: Add file level QL Doc.

37aa6b2

Java: Exclude static initializers from model generation.

2b1423d

Java: Update negative summaries where static initializers has been ex…

91abf79

…cluded.

C#/Java: Add descriptive comment on negative summaries in ExternalFlow.

e8d7266

michaelnebel force-pushed the java/models-io branch from 7537321 to e8d7266 Compare August 29, 2022 12:31

michaelnebel marked this pull request as ready for review August 29, 2022 14:26

michaelnebel requested review from a team as code owners August 29, 2022 14:26

michaelnebel added the no-change-note-required This PR does not need a change note label Aug 29, 2022

michaelnebel mentioned this pull request Aug 30, 2022

Kotlin: Add MaD for stdlib #9783

Merged

tamasvajk reviewed Aug 30, 2022

View reviewed changes

michaelnebel requested a review from tamasvajk August 31, 2022 08:42

tamasvajk approved these changes Aug 31, 2022

View reviewed changes

michaelnebel merged commit 1cb6d78 into github:main Aug 31, 2022

intrigus-lgtm reviewed Aug 31, 2022

View reviewed changes

Java: Update models for commons-io and add negative models. #10170

Java: Update models for commons-io and add negative models. #10170

Uh oh!

Conversation

michaelnebel commented Aug 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 25, 2022

java

Generated file changes for java

Uh oh!

github-actions bot commented Aug 26, 2022

java

Generated file changes for java

Uh oh!

github-actions bot commented Aug 29, 2022

java

Generated file changes for java

Uh oh!

github-actions bot commented Aug 29, 2022

java

Generated file changes for java

Uh oh!

github-actions bot commented Aug 29, 2022

java

Generated file changes for java

Uh oh!

github-actions bot commented Aug 29, 2022

java

Generated file changes for java

Uh oh!

tamasvajk left a comment

Choose a reason for hiding this comment

Uh oh!

tamasvajk Aug 30, 2022

Choose a reason for hiding this comment

Uh oh!

michaelnebel Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

michaelnebel commented Aug 31, 2022

Uh oh!

intrigus-lgtm Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

michaelnebel Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

intrigus-lgtm Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michaelnebel commented Aug 25, 2022 •

edited

Loading