file_scan_directory() does not exclude config folder by default

The config folders can be renamed to something else, so removing config_* folders would not suffice. It would also open up the possibility of stripping out folders that begin with 'config_' and are not related to core. A better option (if it is determined this is necessary at all) would be to add the directories from the $config_directories global.

I'm still not convinced this is necessary though. There may be legitimate reasons to list this directory (for instance when building a deployment tool.) I think it is a better option to leave this to contributed module authors to manage.

better tags.

We need to amend the doxygen to something like "scanning the whole public files directory with file_scan_directory can lead to severe perfomance problems. if you must, use your own subidrectory but it's better not use this function on user uploaded content at all -- rather use the file managed API to keep a record of your files". That's all is needed here and that's a useful addition anyways.

Retagging because I'd still like some more opinions.

Did you actually read what I said above? Any file management module can get the current config directory from the $config_directories global and hide it themselves if they feel it is necessary for their use case. I believe this is the appropriate measure. chx is on the security team and he agrees. I have asked other members of the security team to comment if they feel there is a problem with this.

So... What are we needing to add to the documentation? I'm confused.

Something like this

Um. I think it would be fairly obvious that scanning the entire public files directory would take some time? Why do we need to say this?

Also, if you want to say to use the Managed Files API, it needs to be a link or have a @see, or better yet some explanation of why this is better or what it buys you, or which function to use to scan for files using that API.

And there are problems with punctuation and capitalization in the last line. Should be "module-related files" and "Managed Files API".

Normally, documentation changes are left in the documentation component.

Added documentation to file_scan_directory() to remind people that config_ directories could be included under the right conditions. I also noted how they could make sure their code is more secure by using the right parameters.

Thanks!

This patch has a few typographical issues:

+ * scanned. This will not exclude config_ directories by default, so secure
+ * usage of this API can be assured by either limiting the initial base
+ * directory to a specific folder, limiting the recursion or adding
+ * "^config_" to the nomask option.

a) assured ==> ensured

b) needs a comma after "recursion" in the 3rd line (we use serial commas in the Drupal project by convention)... but see (c) below.

c) And also, as noted above, this is not really accurate. Limiting recursion will not actually help with security, but only with the performance issues noted above. And adding "config" specifically is not going to work for someone who has used a different config directory. So I think we should take out the recursion part, and not talk about "config_" specifically, instead say something more generic about "configuration directories" and "use the nomask option to skip configuration directories".

Awesome, thanks for the great feedback. I'm trying again with another patch (attached).

Looks pretty good! You need a comma before "and" here:

+ * information. Also, the base directory, recurse and min_depth options can 
+ * affect performance by limiting how much of the filesystem has to be
+ * traversed.

And... Maybe it would be clearer to say something like "you can use the ... options to improve performance..."?

I'm getting there! :)

Ok, another patch file attached. Thanks for the feedback/review! :)

Thanks -- text looks good!

There is an extra space at the end of one of the lines though. Most programming editors can be configured to remove or highlight spaces at the ends of lines -- if you are going to continue working on Drupal patches (and I hope you will!), you might want to look for that setting.

D'oh! Ok, removed. Thanks for catching that.

Thanks!

Thanks again everyone! Committed to 8.x.