Opened 10 years ago
Last modified 4 weeks ago
#32544 assigned enhancement
No function exposes all supported MIME types
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Priority: | low | |
Severity: | normal | Version: | 4.2.2 |
Component: | Media | Keywords: | needs-test-info |
Focuses: | Cc: |
Description (last modified by )
wp_get_mime_types()
returns the default WP MIME types, and get_allowed_mime_types()
returns the default WP MIME types + added MIME types through the upload_mimes
filter, but the kicker with the latter is that it is filtered based on the currently authenticated user.
There needs to be a way to retrieve all MIME types supported, both the default and the ones added through the upload_mimes
filter, regardless of who is authenticated.
I understand there are security reasons for not allowing users to *add* (or write) attachments of a given MIME type, but when a user is reading there needs to be a way to retrieve an unrestricted list of all MIME types.
An example use case is below, where all supported MIME types for a service are retrieved from its API and then the subset of types that are both supported by the WordPress install and the service are kept.
function get_service_mime_types() { // the desired function to get un-sanitized MIME types $wp_types = wp_get_all_mime_types(); // get intersection of WP / service MIME types $allowed = array_intersect($wp_types, get_service_mime_types()); $allowed = array_keys($allowed); return $allowed; }
As a work-around, I'm currently using the following, but it's flawed -- if a custom MIME type is added conditionally by user type, it won't be included which is not the desired behavior.
array_merge(wp_get_mime_types(), get_allowed_mime_types())
Attachments (4)
Change History (17)
#4
in reply to:
↑ 3
@
10 years ago
Replying to ipm-frommen:
To me, it is not clear what the desired functionality exactly is.
Since the
upload_mimes
filter passes the current user ID, there is no way to include all MIME types, for every possible user.
I agree. This functionality cannot be achieved without a structural change to the way that additional MIME types are added.
For example, a plugin could use the above filter as well as the user ID like so:
add_filter( 'upload_mimes', 'user_mimes', 10, 2 ); function user_mimes( $mimes, $user_id ) { $user = get_userdata( $user_id ); if ( $user && $user->user_firstname === 'Abe' ) { $mimes[ 'foo' ] = 'text/foo'; } return $mimes; }If I understand your last sentence correctly, you would like to have
'foo'
included in the result of your desiredwp_get_all_mime_types
function. Is this correct? If so, this is not possible.
It is absolutely possible. As stated above, it would require a structural change.
Also, your example code is wrong (there is an endless loop).
Not sure where you're seeing a looping construct...
#5
follow-up:
↓ 6
@
10 years ago
I'm talking about the recursion (i.e., get_service_mime_types
being called within get_service_mime_types
).
Regarding your desired functionality, could you (again) explain in more detail what you want exactly? With the current upload_mimes
filter (and its parameters), I don't see any way to achieve what (I guess) you want. If your structural change implicates removing the $user
ID field from the filter's parameters, this, of course, breaks backwards compatibility.
I'm afraid, I just didn't understand what you would like to have.
#6
in reply to:
↑ 5
@
10 years ago
Replying to ipm-frommen:
I'm talking about the recursion (i.e.,
get_service_mime_types
being called withinget_service_mime_types
).
Gotcha. That would be a typo; pretend the internal call gets a full unfiltered list of all MIME types supported by the service -- maybe via an HTTP request or some cached option.
Regarding your desired functionality, could you (again) explain in more detail what you want exactly? With the current
upload_mimes
filter (and its parameters), I don't see any way to achieve what (I guess) you want. If your structural change implicates removing the$user
ID field from the filter's parameters, this, of course, breaks backwards compatibility.
I was thinking the opposite, actually. Add an additional "override" param which tells the function to unconditionally include all MIME types regardless of user. Short of a complete rework of how MIME types are tracked, this seems like the cleanest solution to me.
I'm afraid, I just didn't understand what you would like to have.
#7
@
9 years ago
- Keywords has-patch has-unit-tests added; needs-patch removed
The patch I attached contains a solution. Like @dan.rossiter said, it's the best solution that doesn't involve refactoring how mime types are handled. This won't change anything unless plugin/theme developers obey the new $ignore_user flag. It works like this:
add_filter( 'upload_mimes', 'user_mimes', 10, 3 );
function user_mimes( $mimes, $user_id, $ignore_user ) {
$mime_types = array( 'test' => 'text/test' );
$user = get_userdata( $user_id );
if ( $ignore_user ) {
$mimes = array_merge( $mimes, $mime_types );
}
else if ( $user && $user->user_firstname === 'Abe' ) {
$mimes = array_merge( $mimes, $mime_types );
}
return $mimes;
}
Basically developers would need to return all of the mime types their code adds if the $ignore_user flag is true.
Then you can get all mime types with this:
$all_mimes = get_allowed_mime_types( null, true );
#8
@
9 years ago
- Owner set to octalmage
- Status changed from new to assigned
Assigning to mark the good-first-bug as "claimed".
See 32544.diff
#9
@
9 months ago
- Keywords needs-testing added
Refreshed the patch and renamed the function so that it is more clear to what it does.
Testing Instructions
- Create a plugin or use your functions file from theme
- Add a custom mime type
- Test the new function by running
get_allowed_mime_types(null, true)
- Verify that the custom mime type is in there
#10
@
4 weeks ago
- Keywords close needs-test-info added; good-first-bug has-patch has-unit-tests needs-testing removed
I'm reading this, and I can't figure out any sense: As @ipm-frommen suggested, the idea is flawed from the beginning; and then reporter proposed a new stunt but did not provide the exact code to prove and test that stunt (to me the comment is like: "do a triple reversed somersault and there you have it")
The current patch pretends that plugin developers, start using that logic to permit a full display of all unfiltered mime types for any use-case that is not even clear. But this is not what the reporter is asking for: he wants full-access regardless of the decisions of individual plugin developers, to restructure (and potentially generate security concerns) the whole function, so user filtering can be completely bypassed on call.
On top of this, the john of 10 years ago, believed that this was a good-first-bug
which is not (in fact, as @ipm-frommen I'm also doubting if implementing the enhancement in this report is a good-idea-at-all
)
Personally, I would preemptively consider this a close
candidate for wontfix
unless the reporter (unlikely, 10 years later) provides additional information about the full use-case (not just the flawed piece of code in a recursive endless loop that was provided in the OP)
#11
follow-up:
↓ 13
@
4 weeks ago
@SirLouen, for someone who openly admits to not understanding the gap in functionality, you certainly have some strong opinions on the matter.
For starters, you make some wild assertions about the security implications of having a function return a read-only superset of MIME types supported in the current deployment with no actual data to support any security concern. This sort of commentary is wildly irresponsible at best.
Additionally, you ask *why* the functionality is needed, even though the initial question spells out a use case for the functionality. For your further edification, the functionality is central to providing a full user experience for Document Gallery (https://wordpress.org/plugins/document-gallery/).
If you have any legitimate questions about the needed functionality, feel free to ask them. But your hyperbolic and colorful language attacking volunteer plugin developers is unwelcome here.
#13
in reply to:
↑ 11
@
4 weeks ago
Replying to dan.rossiter:
If you have any legitimate questions about the needed functionality, feel free to ask them. But your hyperbolic and colorful language attacking volunteer plugin developers is unwelcome here.
My security concerns were completely secondary in all my argument. Don't hang to them for being "offensive" as they are, at best, irrelevant to the whole point. I just say that "they could be potentially concerning" (but not completely concerning). What I was trying to leave clear were the reasons of why this report stalled. Furthermore, I did not expect you to come back after 10 years of a report, ngl. But it's always good to refresh if necessary. The reality is that I'm just triaging and testing, and I admit I can be sometimes a little straightforward to avoid too many rumblings (like this), that happen to be more frequent than ideal. This said, I apologize if it felt wrong for you, and let's set a new path for this stalled enhancement.
First, I need to confirm with you, if you reviewed the last proposed patch, and if it's aligned with your description. From what I've been reviewing, I believe it's not what you are suggesting in the first place, but maybe I'm wrong.
Second, we need to define a full use-case for both pre-testing and post-testing (after the patch), and I believe that currently we don't have the details fully clear. About the plugin you mentioned, can you inform/link in which parts of the code we could see a benefit from this proposal?
To me, it is not clear what the desired functionality exactly is.
Since the
upload_mimes
filter passes the current user ID, there is no way to include all MIME types, for every possible user.For example, a plugin could use the above filter as well as the user ID like so:
If I understand your last sentence correctly, you would like to have
'foo'
included in the result of your desiredwp_get_all_mime_types
function. Is this correct? If so, this is not possible.Also, your example code is wrong (there is an endless loop).