Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

fix: Make File\Extension usable for non-existing files #266

Merged
merged 4 commits into from
Dec 28, 2019

Conversation

edannenberg
Copy link
Contributor

Currently the validator fails if the given filename is not readable. This prevents valid use cases like checking if a user configurable filename has an allowed file extension before creating said file.

As the validator only operates on the given string there is no need for the current readable check.

Also fixes File\ExcludeExtension which suffers from the same issue.

In theory it's a BC breaking change, personally I would consider any code that (ab)uses this validator as a file_is_readable validator to be smelly/buggy.

Currently the validator fails if the given filename is not readable.
This prevents valid use cases like checking if a user configurable
filename has an allowed file extension before creating said file.

As the validator only operates on the given string there is no
need for the current readable check.

Also fixes File\ExcludeExtension which suffers from the same issue.
@weierophinney
Copy link
Member

This is definitely a change in behavior; we have tests that verify that the validator returns false when the file does not exist (which you can see in the test results).

That said, looking through the code, you are absolutely correct - the validators are not testing any of the file contents, just the file name.

For BC purposes, this needs to be opt-in behavior, with the current behavior being the default. We can revisit that when we consider a new major version.

I'll push a change shortly to enable this.

…ion validators

This builds on the original patch. However, to avoid a BC break, this
patch adds a new option to each of the Extension and ExcludeExtension
file validators, `allowNonExistentFile`. By default, this flag is
`false`, preserving the existing behavior where a file must exist in
order to be validated. When `true`, the validator does not automatically
return `false` for non-existent files, and will continue to test its
extension.
@weierophinney
Copy link
Member

I've pushed a change to make this optional behavior, and added documentation on how to enable the option.

@weierophinney weierophinney changed the base branch from master to develop December 28, 2019 02:44
Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

With the change to make the new behavior optional, this can go into the next minor.

@weierophinney weierophinney merged commit cc8805d into zendframework:develop Dec 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants