WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 2 years ago

#19643 reviewing defect (bug)

Allow array for $extra_fields in request_filesystem_credentials

Reported by: griffinjt Owned by: dd32
Milestone: Awaiting Review Priority: normal
Severity: minor Version: 3.0
Component: Filesystem API Keywords: has-patch needs-testing dev-feedback
Focuses: Cc:

Description

The current implementation for passing extra fields through request_filesystem_credentials() does not allow for an array of data to be passed. I came across this issue when trying to process a bulk installation of plugins with my plugin installation class. My patch fixes this from what I can tell and doesn't break anything that I can see from my testing.

Attachments (4)

accept_array_fields.diff (762 bytes) - added by griffinjt 6 years ago.
allow array of data to be passed through $extra_fields
updated_array.diff (911 bytes) - added by griffinjt 6 years ago.
updated to fix notices
extra-fields-callback.diff (1.7 KB) - added by griffinjt 6 years ago.
callback function to accept any type of extra field
19643.patch (1.2 KB) - added by kurtpayne 5 years ago.
Alternate patch - handles multi-dimensional arrays

Download all attachments as: .zip

Change History (25)

@griffinjt
6 years ago

allow array of data to be passed through $extra_fields

#1 @griffinjt
6 years ago

  • Keywords needs-testing added

#2 @dd32
6 years ago

That'll issue a PHP Notice on any $extra_fields which aren't in the incoming $_POST array. That's why there's a isset() call there

@griffinjt
6 years ago

updated to fix notices

#3 @griffinjt
6 years ago

It's late - forgive me. :-) That should fix the error warnings now and still works fine as far as I have tested.

#4 @GaryJ
6 years ago

Instead of doing an is_array() check and duplicating the echo line, why not cast $_POST['field'] to an array and just have the foreach?

#5 @griffinjt
6 years ago

Doing that won't allow for the input name attribute to be set as an array. That's why I had the two echo lines.

#6 @griffinjt
6 years ago

Any word on this? Again with testing I haven't run into any bugs and it works perfectly for passing an array of data from a previous post entry.

#7 @dd32
6 years ago

  • Keywords dev-feedback added

Any word on this?

Most of us are taking a break after 3.3's release. 3.4 dev will be starting after the holiday season and people will look over it then most likely. Have patience.

#8 @griffinjt
6 years ago

Ok,cool. No worries!

#9 @wpsmith
6 years ago

  • Cc travis@… added

+1

#10 @Otto42
6 years ago

  • Cc Otto42 added

#11 @griffinjt
6 years ago

Ok, here's my feeble attempt at providing a solution that will catch any type of value submitted via $extra_fields and parse it correctly within the input field. Because my knowledge of recursion is shoddy at best, I came up with this function instead.

All it does is sanitize and store all the data passed via $extra_fields into an array and then builds a query string based on that data. Then I just run through the query string and make input fields.

It works in all my testing. I don't know if this is the -best- method, but it is a -working- method that can at least get the ball rolling so that no data is lost when attempting to pass it once the user hits "Proceed".

@griffinjt
6 years ago

callback function to accept any type of extra field

#12 @griffinjt
6 years ago

Just updated the patch to use (the newly discovered to me) WP's build_query function. Would love to get some feedback on this - no more losing data when having to enter FTP credentials. :-)

#13 @griffinjt
5 years ago

  • Type changed from enhancement to defect (bug)

@kurtpayne
5 years ago

Alternate patch - handles multi-dimensional arrays

#14 @kurtpayne
5 years ago

  • Cc kurtpayne added

Submitting an alternate patch. This will handle multi-dimensional arrays, too. E.g.

<input type="hidden" name="field1[]" value="value1" />
<input type="hidden" name="field1[]" value="value2" />
<input type="hidden" name="field1[]" value="value3" />
<input type="hidden" name="field2[deep][array][value]" value="value4" />
<input type="hidden" name="field3[][][]" value="value5" />
<input type="hidden" name="field4" value="value6" />

And the corresponding php:

$form_fields = array ( 'field1', 'field2', 'field3', 'field4' );
request_filesystem_credentials($url, $method, false, false, $form_fields) );

I'm testing this with Otto's test filesystem plugin.

#15 @kurtpayne
5 years ago

  • Severity changed from normal to minor
  • Version changed from 3.3 to 3.0

#16 @wonderboymusic
3 years ago

#28207 was marked as a duplicate.

#17 @jrf
3 years ago

Wondering what the status is of this. Ran into this bug myself as well.

Code-wise, I'm in favour of @kurtpayne's patch, through I'd prefer to see the array_print as a proper function to stay in line with most other WP code.

#18 @DrewAPicture
3 years ago

  • Owner set to dd32
  • Status changed from new to reviewing

@dd32: Care to follow up here?

#19 @dd32
3 years ago

Before considering any changes here, we'll be best off seeing what happens with the inline-install #29820.
This screen is mostly going to disappear, or need to be rewritten for that I believe.

#20 @jrf
3 years ago

@dd32 I'm not sure I understand why as file read/writes can need to be done quite legitimately outside of the context of plugin installation. This is about the FileSystem API, not necessarily about plugin installation/upgrade etc.

#21 @jrf
2 years ago

What can be done to move this forward ?

Note: See TracTickets for help on using tickets.