Custom Mime Type filter problem in upload.php list view

[csschris]

The issue is that the option values in the select box for attachment-filters are being passed through urlencode on page render and form submission.

I describe the problem with more detail ​here

This only happens in list view.

[birgire]

As also reported independently here:

​http://wordpress.stackexchange.com/questions/166530/post-mime-types-filter-not-working-in-list-mode

there's a potential problem when filtering custom post mime types, in the list mode.

When we register a custom post mime type, as suggested:

add_filter( 'post_mime_types', function( $post_mime_types ) {
    $post_mime_types['application/pdf'] = array( __( 'PDFs' ), __( 'Manage PDFs' ), _n_noop( 'PDF <span class="count">(%s)</span>', 'PDFs <span class="count">(%s)</span>' ) );
    return $post_mime_types;
});

then it will add the following option to the select box for attachments filters:

<option value="post_mime_type:application%2Fpdf" selected="selected">PDF (1)</option>

When we submit this option, the relevant GET request becomes:

&attachment-filter=post_mime_type%3Aapplication%252Fpdf

The reason for this is the urlencode part:

'<option value="post_mime_type:' . urlencode( $mime_type )

Check out this source line: https://core.trac.wordpress.org/browser/tags/4.0/src/wp-admin/includes/class-wp-media-list-table.php#L73

This wasn't a problem in WP 3.9 because it wasn't encoded:

$type_links[$mime_type] = "<a href='upload.php?post_mime_type=$mime_type'$class>"

See this code line in version 3.9: https://core.trac.wordpress.org/browser/tags/3.9/src/wp-admin/includes/class-wp-media-list-table.php#L67

I wonder if we could use esc_attr() instead of urlencode():

'<option value="post_mime_type:' . esc_attr( $mime_type )

so the HTML of the option becomes:

<option value="post_mime_type:application/pdf" selected="selected">PDF (1)</option>

and the GET request becomes:

&attachment-filter=post_mime_type%3Aapplication%2Fpdf

which works as expected.

Ps:

Another idea would be to use the sanitize_mime_type core function:

'<option value="post_mime_type:' . sanitize_mime_type( $mime_type )

where:

function sanitize_mime_type( $mime_type ) {
        $sani_mime_type = preg_replace( '/[^-+*.a-zA-Z0-9\/]/', '', $mime_type );
        /**
         * Filter a mime type following sanitization.
         *
         * @since 3.1.3
         *
         * @param string $sani_mime_type The sanitized mime type.
         * @param string $mime_type      The mime type prior to sanitization.
         */
        return apply_filters( 'sanitize_mime_type', $sani_mime_type, $mime_type );
}

[birgire]

sorry, hopefully this is the correct .diff file, not the txt version as my previous ones ;-)

[birgire]

sorry, hopefully this is the correct .diff file, not the txt version as my previous ones ;-)

[wonderboymusic]

In 30661:

Use sanitize_mime_type() instead of urlencode() when string-building <option> values in ->get_views() for media list tables.

urlencode() was introduced in [28553] via _duck and yours truly.

Got weirder after [29625].

Props birgire.
Fixes #30123.

[SergeyBiryukov]

#30520 was marked as a duplicate.

[mdgl]

Note that the chosen solution here using sanitize_mime_type() breaks when presented with a comma-separated list of MIME types which is a valid return from get_post_mime_types(). The originally suggested fix of using esc_attr() works properly and seems more correct to me. See #30788.

[birgire]

@mdgl that's interesting.

To handle a comma sepereated list of mime types, like

application/x-pdf, application/pdf 

I think it would be logical to extend sanitize_mime_type to support this.

For example by adding a single comma (,) into the preg_replace part of that function.

So instead of this line of the sanitize_mime_type() function:

$sani_mime_type = preg_replace( '/[^-+*.a-zA-Z0-9\/]/', '', $mime_type );

we could instead use this:

$sani_mime_type = preg_replace( '/[^-+*.,a-zA-Z0-9\/]/', '', $mime_type );

[mdgl]

Hello birgire, yes, changing sanitize_mime_type() as you suggest would fix the problem, but I don't think this is the right way to go. The comma is not valid in (simple) MIME type names [see ​https://www.ietf.org/rfc/rfc2045.txt and ​http://www.rfc-editor.org/rfc/rfc2231.txt] and by changing this function we might be introducing problems elsewhere.

The bug here is simply that the value needs escaping as it is being used inside a HTML attribute, so I think your original suggestion of using esc_attr() is the correct solution.

[birgire]

Allowing the extra comma in sanitize_mime_type() would be like working with sanitize_mime_types() instead.

But if not, then I guess we could use the esc_attr() function as suggested ;-)

ps: I just noticed that your ticket is closed and fixed ;-)