Skip to content

ValueError for empty path in stream code #5902

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 1 commit into from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jul 29, 2020

Not sure how to go about the message content, I changed it to path as it seems more general, but can revert back to "filename"

@nikic
Copy link
Member

nikic commented Jul 29, 2020

AppVeyor failures legit.

@Girgias Girgias force-pushed the stream-value-error branch from 6c793bd to 31b99d2 Compare July 29, 2020 16:09
@Girgias Girgias force-pushed the stream-value-error branch from 31b99d2 to 367f430 Compare July 29, 2020 21:58
@@ -2066,7 +2066,7 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod
}

if (!path || !*path) {
php_error_docref(NULL, E_WARNING, "Filename cannot be empty");
zend_value_error("Path cannot be empty");
Copy link
Member

Choose a reason for hiding this comment

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

I believe we ended up using TypeError for this case everywhere else (because it's often checked by zpp).

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought TypeErrors were just in case the path contained a null byte?

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked again and indeed we use Value error for the empty case: https://heap.space/search?project=php-src&full=%22cannot+be+empty%22&defs=&refs=&path=&hist=&type=c

Copy link
Member

Choose a reason for hiding this comment

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

Okay, thanks for checking.

@php-pulls php-pulls closed this in c3105a1 Jul 31, 2020
@Girgias Girgias deleted the stream-value-error branch July 31, 2020 12:40
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.

2 participants