Skip to content

Fix partial path traversal Java example Again#12735

Merged
atorralba merged 3 commits into
github:mainfrom
JLLeitschuh:doc/JLL/fix-partial-path-documentation
Apr 3, 2023
Merged

Fix partial path traversal Java example Again#12735
atorralba merged 3 commits into
github:mainfrom
JLLeitschuh:doc/JLL/fix-partial-path-documentation

Conversation

@JLLeitschuh
Copy link
Copy Markdown
Contributor

The original wouldn't compile, and the fix made by #11899 is sub-optimal.
This keeps the entire comparision using the Java Path object, which is optimal.

Signed-off-by: Jonathan Leitschuh Jonathan.Leitschuh@gmail.com

@github-actions
Copy link
Copy Markdown
Contributor

QHelp previews:

Copy link
Copy Markdown
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

Thanks. Quite the subtlety with Path#startsWith compared to String#startsWith. I had to quickly test it locally to actually understand the difference.

Do you think it makes sense to add this "prime example" as a test, so that we validate that the recommended solution isn't flagged by the query, now and in the future?

Comment thread java/ql/src/Security/CWE/CWE-023/PartialPathTraversalRemainder.inc.qhelp Outdated
@JLLeitschuh
Copy link
Copy Markdown
Contributor Author

Do you think it makes sense to add this "prime example" as a test, so that we validate that the recommended solution isn't flagged by the query, now and in the future?

Yes, I agree it should be added.

@JLLeitschuh JLLeitschuh force-pushed the doc/JLL/fix-partial-path-documentation branch from 6da65b7 to 90d80ed Compare March 31, 2023 19:26
JLLeitschuh and others added 3 commits March 31, 2023 23:36
The original wouldn't compile, and the fix made by github#11899 is sub-optimal.
This keeps the entire comparision using the Java `Path` object, which is optimal.

Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>
….inc.qhelp

Co-authored-by: Tony Torralba <atorralba@users.noreply.github.com>
The original wouldn't compile, and the fix made by github#11899 is sub-optimal.
This keeps the entire comparision using the Java `Path` object, which is optimal.

Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>
@JLLeitschuh JLLeitschuh force-pushed the doc/JLL/fix-partial-path-documentation branch from 90d80ed to 0d774a6 Compare April 1, 2023 03:36
@atorralba atorralba added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Apr 3, 2023
@mchammer01 mchammer01 self-requested a review April 3, 2023 09:00
Copy link
Copy Markdown
Contributor

@mchammer01 mchammer01 left a comment

Choose a reason for hiding this comment

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

This LGTM from an editorial point of view (only reviewed the qhelp update) ⚡

@atorralba atorralba merged commit 6331c37 into github:main Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Java ready-for-doc-review This PR requires and is ready for review from the GitHub docs team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants