Skip to content
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

Array cast pass taints #6810

Merged
merged 1 commit into from
Nov 4, 2021
Merged

Array cast pass taints #6810

merged 1 commit into from
Nov 4, 2021

Conversation

orklah
Copy link
Collaborator

@orklah orklah commented Nov 3, 2021

This makes array casts taint sensitive and fix #5550

@weirdan
Copy link
Collaborator

weirdan commented Nov 3, 2021

The issue with invalid data returned from the data provider should have been caught by Psalm, but somehow it wasn't. Even though one of the elements is missing error_message key Psalm thinks the returned array is non-empty-array<string, array{0: non-empty-string, error_message: non-empty-string}>. I wonder if there's any threshold after which array elements stop contributing to the inferred shape.

@orklah
Copy link
Collaborator Author

orklah commented Nov 3, 2021

I wonder if there's any threshold after which array elements stop contributing to the inferred shape.

If I remember correctly, it's this one:

if ($item_key_value !== null && count($array_creation_info->property_types) <= 100) {

@weirdan
Copy link
Collaborator

weirdan commented Nov 3, 2021

Psalm uses at most 101 elements to infer array shape: https://psalm.dev/r/5f192e37bf
And at most 102 elements to infer array value type: https://psalm.dev/r/62d9666fdc
Elements after 102th are ignored: https://psalm.dev/r/6462ff9bcc

This doesn't look right. While it's understandable that we cannot allow unbounded shapes, we probably should still be able to use all elements to infer array value/key type (as we actually process all elements).

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/5f192e37bf
<?php

$_a = [
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 10
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 20
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 30
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 40
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 50
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 60
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 70
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 80
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 90
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 100
            "aaaa",
];
/** @psalm-trace $_a */;
Psalm output (using commit 35868c0):

INFO: Trace - 16:24 - $_a: array{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, "aaaa"}
https://psalm.dev/r/62d9666fdc
<?php

$_a = [
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 10
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 20
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 30
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 40
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 50
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 60
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 70
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 80
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 90
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 100
            1, // 101
            "aaaa",
];
/** @psalm-trace $_a */;
Psalm output (using commit 35868c0):

INFO: Trace - 17:24 - $_a: non-empty-list<"aaaa"|1>
https://psalm.dev/r/6462ff9bcc
<?php

$_a = [
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 10
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 20
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 30
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 40
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 50
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 60
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 70
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 80
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 90
            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 100
            1, 1, // 102
            "aaaa",
];
/** @psalm-trace $_a */;
Psalm output (using commit 35868c0):

INFO: Trace - 17:24 - $_a: non-empty-list<1>

@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Nov 3, 2021
@orklah orklah merged commit bf99345 into vimeo:master Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typecasting to array breaks taint flow
2 participants