Skip to content

Make null byte error a ValueError #6094

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

Closed
wants to merge 2 commits into from

Conversation

nikic
Copy link
Member

@nikic nikic commented Sep 8, 2020

Currently we treat paths with null bytes as a TypeError, which is semantically an odd choice, especially as we treat empty paths as ValueError. We do this because the error is generated by zpp and it's easier to always throw TypeError there.

This changes the zpp implementation to throw a TypeError only if the type is actually wrong and throw ValueError for null bytes. The error message is also split accordingly, to be more precise.

Test updates here are very much WIP.

@nikic
Copy link
Member Author

nikic commented Sep 8, 2020

cc @kocsismate @Girgias @cmb69

@nikic nikic force-pushed the null-byte-value-error branch from bf9b4cc to 227988d Compare September 8, 2020 10:19
@cmb69
Copy link
Member

cmb69 commented Sep 8, 2020

Makes perfect sense to me (slightly harder for the docs, but that shouldn't be a stopper).

@nikic nikic force-pushed the null-byte-value-error branch from 227988d to b4ba43c Compare September 8, 2020 11:08
@Girgias
Copy link
Member

Girgias commented Sep 8, 2020

LGTM, this would allow us to use the PATH ZPP check in more place as before the usage of path could be quite confusing.

@nikic nikic force-pushed the null-byte-value-error branch from b4ba43c to 92059ce Compare September 8, 2020 12:17
@php-pulls php-pulls closed this in 7e339a3 Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants