Conversation
|
I found these snippets: https://psalm.dev/r/0d284c6f48<?php
$foo = (object) null;
$bar = (int) $foo;
/** @psalm-trace $bar */;
echo $bar; |
18fbdb8 to
8d786b6
Compare
|
Feedback appreciated :-) Could someone please help with the failing taint tests? |
|
There's a lot going on here that I don't quite understand, could you add a description of the issue you're trying to solve?
If taint tests (or unused variable tests) are failing, you probably messed up If you have this code: $foobar = $_GET["foobar"];
if (is_string($foobar)) {
echo $foobar; // XSS vulnerability!
}Then what happens (at a high level) is that on line 1 |
Summary: this PR adds correct support for typecasting to int/float.
Yes, thanks. That was it, the first commit that just unified the code messed it up. Reverted it now (will make it look nice later). |
|
I found these snippets: https://psalm.dev/r/0d284c6f48<?php
$foo = (object) null;
$bar = (int) $foo;
/** @psalm-trace $bar */;
echo $bar;https://psalm.dev/r/9e15ff2d1f<?php
$foo = array( 'hello' );
$data = (int) $foo;
/** @psalm-trace $data */;
echo $data; |
I think the array one that's failing is the one I mentioned previously, looks like the rest of them are some taint tests that are still failing. |
@orklah What do you think about adding |
|
Yeah, that makes sense, we should try to keep this rule consistent |
c2e1bea to
683f8d2
Compare
|
@AndrolGenhald added it as |
|
This PR will also fix the pending half of #6966 |
b643855 to
893a6f8
Compare
|
@orklah Ready to be reviewed & merged finally, I figured out the taint stuff. Also cherry-picked that other PR, removed the code that is better handled with this PR and adjusted tests for 4.x |
893a6f8 to
aebe936
Compare
|
@AndrolGenhald rebased and issues fixed, except "ComplexMethod" in these 2 functions as it doesn't make sense to rework this now, when this has to be modified again in #8486 and I think fixing these 2 issues now, will just make it harder for the followup work. |
bdb66e9 to
5ab90d4
Compare
Agreed, could you Also, it looks like there's now a regression in the bitwiseAssignment test. |
94f56a7 to
6986ad8
Compare
…meo#6966) (vimeo#7071) * Fixed vimeo#6966 * Only accept >= 0 values for mode argument in round() * Made round() only return float or literal float values and remove unneeded test * Registered RoundReturnTypeProvider * Updated cast analyzer to handle single string literal int values as literal ints * Fixed psalm errors * Fix invalid property accesses * Addressed comments * Added Tests * Marked RoundReturnTypeProvider as internal * Fixed CS
6986ad8 to
d69be4b
Compare
|
Out of curiosity, what was the issue with the broken bitwiseAssignment test? |
|
@AndrolGenhald when I used TIntRange(0, 1) psalm reduced this to "int" and reported "unused psalm suppress". So I changed it to literals: |
|
Huh, weird. Guess there's other bugs around causing that then. @orklah everything look good to you? |
|
Code seems good, could you add some tests for RiskyCast please? |
|
Thanks! |
e.g. https://psalm.dev/r/0d284c6f48 gives
Warning: Object of class stdClass could not be converted to intSince by default there cannot be any "PossiblyInvalidCasts", as we always get 0/0.0 in error case, added mostly unwanted typecasts that are a leading cause of off-by-one errors as PossiblyInvalidCasts
Fix: #6966 (the remaining half + cherry-pick the other half from master to 4.x)