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

Add warning for list_files function and related #704

Open
ovidiul opened this issue Dec 15, 2021 · 6 comments
Open

Add warning for list_files function and related #704

ovidiul opened this issue Dec 15, 2021 · 6 comments
Milestone

Comments

@ovidiul
Copy link

ovidiul commented Dec 15, 2021

What problem would the enhancement address for VIP?

I've recently been doing a code review and noticed the list_files function had no warning attached. On VIP Filesystem, the list_files, scandir, opendir will return empty result sets or false, so we should probably highlight this to customers to avoid unexpected results.

Describe the solution you'd like

A warning should be added when the following functions are coded:

  • list_files - WP
  • scandir - PHP
  • opendir - PHP

What code should be reported as a violation?

$files = list_files( $folder, 2 );

What code should not be reported as a violation?

Additional context

@GaryJones
Copy link
Contributor

@GaryJones GaryJones added this to the 2.3.4 milestone Jan 18, 2023
@GaryJones
Copy link
Contributor

I've tested this on a VIP test site, and I'm getting the expected results for each of them, so I will close this out.

cc @yolih and @raamdev, who I think had worked on docs around this that may need to be reverted/updated.

@GaryJones GaryJones closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2023
@GaryJones GaryJones removed this from the 2.3.4 milestone Mar 5, 2023
@yolih
Copy link
Contributor

yolih commented Mar 6, 2023

The Docs currently state:

scandir(), list_files(), and opendir() will not work as expected, and will instead return either an empty array or false and trigger a PHP Warning.

So I believe we are in sync with the VIP Coding Standards rules for warnings. Thanks for cross-ping!

@jrfnl
Copy link
Collaborator

jrfnl commented Mar 7, 2023

@yolih @GaryJones I have a feeling there is some miscommunication going on here.

When I read @GaryJones' message, I get the impression he got proper correct file listings, not an empty array. @GaryJones is that correct ?
If so, that would mean the docs would need to be updated to remove the warning about those functions ?

@GaryJones
Copy link
Contributor

Correct. On my VIP test site, the three functions worked as expected. I can get it verified, and if so, or docs need fixing.

@raamdev
Copy link
Member

raamdev commented Mar 9, 2023

The docs are referring to file listing functions on wp-content/uploads/ paths on VIP, which utilize the VIP Filesystem.

I've tested and confirmed that the following is true when those functions are used on wp-content/uploads/:

scandir(), list_files(), and opendir() will not work as expected, and will instead return either an empty array or false and trigger a PHP Warning.

I will reopen this issue with the caveat that we should produce a warning indicating that if those functions are being used on a path that points to the wp-content/uploads/ directory, they will not work as intended.

@raamdev raamdev reopened this Mar 9, 2023
@GaryJones GaryJones added this to the 2.4.0 milestone Mar 9, 2023
@GaryJones GaryJones modified the milestones: 2.4.0, 3.x Aug 21, 2023
@rebeccahum rebeccahum modified the milestones: 3.0.1, 3.x May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants