Skip to content

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented May 13, 2024

Add more defensive coding against incorrect stack pointers being passed.

It is common to pass the result of a call to File::findPrevious() or File::findNext() to functions expecting a stack pointer, but these File functions can return false, which would be juggled to 0 when used in the typical isset($tokens[$stackPtr]) checks. This would then lead to that check passing, while the value should have been rejected, as the method may now try to act on a completely different token than intended (and more defensive coding should have been added to the originating sniff).

Adding a preliminary check to make sure the received parameter is an integer prevents this problem and should surface any such bugs in sniffs using the updated PHPCSUtils methods.

Includes tests.

@jrfnl jrfnl added this to the 1.1.0 milestone May 13, 2024
@jrfnl jrfnl force-pushed the feature/add-more-defensive-coding-typeerrors branch 2 times, most recently from 4769b6a to 68e4394 Compare May 13, 2024 11:11
@jrfnl jrfnl force-pushed the feature/add-more-defensive-coding-typeerrors branch 2 times, most recently from 38ec4cb to b03bf36 Compare May 14, 2024 06:39
@jrfnl jrfnl force-pushed the feature/start-using-the-new-exceptions branch from 10aa322 to 5680552 Compare May 14, 2024 06:53
@jrfnl jrfnl force-pushed the feature/add-more-defensive-coding-typeerrors branch from b03bf36 to da71d5a Compare May 14, 2024 06:53
@jrfnl jrfnl force-pushed the feature/start-using-the-new-exceptions branch 2 times, most recently from 18fb796 to 1c9176d Compare May 20, 2024 18:28
Base automatically changed from feature/start-using-the-new-exceptions to develop May 20, 2024 18:50
@jrfnl
Copy link
Member Author

jrfnl commented May 20, 2024

Rebased without changes, other than squashing the "catch change/test" commit into the main commit.
Marking as ready as #598 and #599 have been merged now. Merging once the build passes.

@jrfnl jrfnl force-pushed the feature/add-more-defensive-coding-typeerrors branch from da71d5a to c15cf06 Compare May 20, 2024 18:55
@jrfnl jrfnl marked this pull request as ready for review May 20, 2024 18:55
Add more defensive coding against incorrect stack pointers being passed.

It is common to pass the result of a call to `File::findPrevious()` or `File::findNext()` to functions expecting a stack pointer, but these `File` functions can return `false`, which would be juggled to `0` when used in the typical `isset($tokens[$stackPtr])` checks.
This would then lead to that check passing, while the value should have been rejected, as the method may now try to act on a completely different token than intended (and more defensive coding should have been added to the originating sniff).

Adding a preliminary check to make sure the received parameter is an integer prevents this problem and should surface any such bugs in sniffs using the updated PHPCSUtils methods.

Includes tests.
@jrfnl jrfnl force-pushed the feature/add-more-defensive-coding-typeerrors branch from c15cf06 to 33541ff Compare May 20, 2024 19:00
@jrfnl jrfnl merged commit 35bad87 into develop May 20, 2024
@jrfnl jrfnl deleted the feature/add-more-defensive-coding-typeerrors branch May 20, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant