Skip to content

Java: convert remaining java-code-scanning.qls query tests to .qlref #19842

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

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

d10c
Copy link
Contributor

@d10c d10c commented Jun 23, 2025

Example prior work: #18848 #19817

  • Does a straightforward conversion of existing .ql-based inline expectations tests to .qlref/utils/test/InlineExpectationsTestQuery.ql-based ones, similar to the above PRs.
  • In the case of CleartextStorageCookie, there was no test, so I created one here.
  • Since the diff-informed consistency check (--check-diff-informed) runs on .qlref-based tests, the UnsafeDeserialization and PolynomialReDoS tests were newly failing that check.
    • In the case of UnsafeDeserialization, the fix was to add a missing getASelectedSinkLocation override.
    • In the case of PolynomialReDoS, the fix was to disable diff-informed support, as the query was failing the consistency check due to a substring of a regex being shown in the results, which does not have a corresponding Location value.

@github-actions github-actions bot added the Java label Jun 23, 2025
@d10c d10c added the no-change-note-required This PR does not need a change note label Jun 23, 2025
@d10c d10c force-pushed the d10c/convert-java-tests-to-qlref branch 2 times, most recently from 3b874a0 to 4a835f9 Compare June 24, 2025 12:43
d10c added 26 commits June 24, 2025 16:41
Also, split off into separate directory from JndiInjectionTest because their $Alerts were interfering with each other.
Leaves ReDoS.ql unmodified since it's not a dataflow query; just moves it to its own directory.
d10c added 5 commits June 24, 2025 16:42
It's a non-path query, so the InlineExpectationsTest postprocessor doesn't do anything.
This fixes the failing diff-informed consistency check.
This is because it was failing the diff-informed consistency check, and like other ReDoS queries (Python?) the query tries to be helpful by showing a substring of a regex, which has a `hasLocation(...)` (intensional) but no corresponding `getLocation()` (extensional). Until the location overrides get updated to support `hasLocation`-based locations, it's probably best to turn off diff-informed support.
@d10c d10c force-pushed the d10c/convert-java-tests-to-qlref branch from 4a835f9 to a49999d Compare June 24, 2025 14:47
Given that it's a non-path-problem dataflow query, the InlineExpectationsTest is not as useful.
@d10c d10c force-pushed the d10c/convert-java-tests-to-qlref branch from d6f8ec3 to 6904461 Compare June 24, 2025 16:12
@d10c d10c requested a review from aschackmull June 24, 2025 16:13
@d10c d10c marked this pull request as ready for review June 24, 2025 16:14
@d10c d10c requested a review from a team as a code owner June 24, 2025 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant