Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A moderate documentation issue remains because the example claims a configurable access-path limit of 8, while the implementation still caps paths at the language default of 5, so the example must be corrected or qualified.
1 open issue
What changed in this PR
Updates the Java/Kotlin data-flow guide to document the access path limit, its default behavior, and configuration options.
Changes:
- Adds
accessPathLimitto the optionalDataFlow::ConfigSigpredicates. - Documents the default limit of 5 and its effect on flow discovery.
- Provides an example for configuring a higher limit and notes the performance trade-off.
| File | Description |
|---|---|
docs/codeql/codeql-language-guides/analyzing-data-flow-in-java.rst |
Documents access path limits and configuration in the Java/Kotlin data-flow guide. |
Review effort: Lite (auto)
Comment on lines
+232
to
+235
| int accessPathLimit() { result = 8 } | ||
| } | ||
|
|
||
| A higher limit can find more flows, but the analysis takes more time and memory. |
Author
aschackmull
added
the
ready-for-doc-review
Contributor
|
Looks fine to me, but I think we'll want a docs review as well. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #23042.
The access path limit (
accessPathLimit(), default 5 in the shared data flow library) decides which flows through fields, collections and maps can be found, but the data flow guides do not mention it. A flow that needs a longer path is dropped without a warning.This PR changes
analyzing-data-flow-in-java.rstin two places:accessPathLimitto the list of optional configuration predicates.DataFlow::ConfigSig.Checking the text. I used CodeQL 2.27.4 (
codeql/java-all9.3.4,codeql/dataflow2.1.17) on two programs that pass a tainted value through five and six nested lists. With the default limit, only the five-list flow is found. Withint accessPathLimit() { result = 8 }in the configuration, the six-list flow should be found, as it is on 2.26.2, but on 2.27.4 it is not, because raising the limit currently has no effect (#23041). The text documents the intended behaviour. If capping the limit at the language default turns out to be intended, I will change the section to say that a configuration can only lower it.The limit lives in the shared library, so the same section could be added to the other languages' data flow guides. I kept this PR to Java and can extend it if that is wanted.