WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 17 months ago

#20716 assigned task (blessed)

Control how and when request_filesystem_credentials outputs creds form

Reported by: griffinjt Owned by: dd32
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4
Component: Filesystem API Keywords: has-patch
Focuses: Cc:

Description

When using WP_Filesystem in a plugin or other external app, you can initialize it this way:

if ( false == ( $creds = request_filesystem_credentials( $url, $method, false, false, $form_fields ) ) ) {
    return true;
}

Makes sense. If the method isn't direct or we don't have the credentials we need, request_filesystem_credentials outputs a form and returns false. Then we check again to make sure the credentials can be verified, and it not, we continue to output the form until the credentials are good.

if ( ! WP_Filesystem( $creds ) ) {
    request_filesystem_credentials( $url, $method, true, false, $form_fields );
    return true;
}

request_filesystem_credentials arbitrarily outputs the form whether you want it to or not, unless you explicitly filter the request_filesystem_credentials function itself. Using this in conjunction with Ajax, you have to use output buffering to catch the output and return it to the Ajax script for processing or else you get errors. This definitely isn't ideal.

I'd suggest storing the form in a variable and possibly pass a parameter that determines whether or not we want to actually output the form or not. Or, if there is another (better) way, that's cool too. Just some way to help request_filesystem_credentials determine how we want to interact with credentials should we need them.

Attachments (1)

20716.diff (1.2 KB) - added by jeremyfelt 18 months ago.

Download all attachments as: .zip

Change History (7)

comment:1 @griffinjt3 years ago

Err, should be === instead of == in that first function. Typo.

comment:2 @dd3218 months ago

  • Keywords needs-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 3.7

This is needed for #22704 specifically for [25228]

Anyone got a patch? :)

@jeremyfelt18 months ago

comment:3 @jeremyfelt18 months ago

  • Keywords has-patch added; needs-patch removed

I think 20716.diff addresses the issue here. It introduces an output_request_filesystem_credentials filter that defaults to true. If set to false, then it returns false for request_filesystem_credentials() rather than outputting the form.

If any of the output is needed for some reason, then a different approach would be needed. We could also throw in a DOING_AJAX check so that the filter doesn't need to be set for some requests.

comment:4 @nacin18 months ago

  • Owner set to dd32
  • Status changed from new to assigned
  • Type changed from enhancement to task (blessed)

comment:5 @dd3217 months ago

It seems that we'd be better to split most of the logic before the form into a separate function, which request_filesystem_credentials() calls. We can then stop calling request_filesystem_credentials() directly, and call that helper instead.

comment:6 @johnbillion17 months ago

  • Milestone changed from 3.7 to Future Release

Punting as per dev chat and dd32's comment.

Note: See TracTickets for help on using tickets.