Allow adding headers to get_plugin_data

[strider72]

It would be nice for plugin authors if we could add headers to what is searched for in get_plugin_data. The attached patch integrates this as a straightforward filter.

For example, an author might add a header that states the minimum WP version, or perhaps a URL for version checking. With this filter he could just drop that info into an easily accessed plugin header instead of hard-coding it somewhere further down. (Beyond simple location, this much simplifies certain processes, such as a plugin with which other plugins can make themselves compatible.)

The attached patch modifies get_plugin_data(). Default output is identical to existing, but you can now do the following filter:

function filter_plugin_data_headers( $extra_params, $plugin_file ) {
	if ( __FILE__ == $plugin_file ) {
		$extra_params['NewHeader'] = 'New Plugin Header';
	}
	return $extra_params;
}
add_filter( 'plugin_data_headers', 'filter_plugin_data_headers', 10, 2 );

In the above, the plugin could have:

New Plugin Header: just testing

and get_plugin_data() would return "just testing" as $plugin_data['NewHeader']

NOTE: This is explicitly written so that a plugin can *not* overwrite existing standard headers. That is, it is not possible to overwrite the Name, Description, etc. of another plugin, as that might pose the risk for mischief. You can only use this to add a new header, which will be returned as part of the get_plugin_data array.

[strider72]

adds filter to get_plugin_data() so you can add new headers to search for

[strider72]

Just a note: I am aware of the similar patch at ​http://trac.wordpress.org/ticket/4048 , but the protests against that one seem to mainly have been the potential to "hack" other plugin by changing their headers. So again, this patch is explicitly designed to only allow you to add a new header to be returned, not to alter existing ones in any way.

[strider72]

Another idea is that perhaps it would be best to only allow the plugin to specify the header, e.g. "New Plugin Header", and the array key for get_plugin_data would be auto-generated from that, e.g. $plugin_data['NewPluginHeader]. Might make it more difficult for two plugins using this from interfering with each other. I'm not sure how this would best work though. Perhaps the key should just be the unaltered Plugin header, e.g. $plugin_data['New Plugin Header'] ?

[Denis-de-Bernardy]

yes, yes, yes! this would be sooooo much better than needing to re-open each plugin file each time an extra field gets added.

[strider72]

Updated the patch so that the key in the resulting $plugin_data array equals the actual name of the Plugin Header.

This change prevents potential conflicts wherein two plugins might define a different but similar header, but then use the exact same key in the resulting array.

[strider72]

changes the way array keys works as described in above comment

[strider72]

If we do this, it should apply to Themes as well.

[westi]

This looks like a good idea.

We definitely need to ensure the changes are additive.

We should also support the same for themes so could you do a patch for that too?

I think the plugin_data_headers filter should be plugin specific to encourage correct programming with a helper function for registering for it like the activate/deactivate ones.

That should ensure that plugins are written to use it correctly and saves the code you have to check in the action for it being for you.

[strider72]

1) The changes are additive -- I do a merge_array giving existing headers priority.

2) Let's get Plugins solid and then I'll go in and do the same for Themes.

3) By "plugin specific" you mean it should only work if it specifies the plugin to check? I say resoundingly, NO. That would defeat the ability to make my plugin work with "whatever other plugins are designed to be compatible". i want to be able to make code that says "For each plugin that has Header X, do this...."

Not sure how "plugin specific" ensures "correct programming"

I don't understand what you mean about a helper function. Why do this? Isn't the filter much simpler? K.I.S.S.

"... and saves the code you have to check in the action for it being for you."

I don't understand that statement at all.

A good example of this plugin is a 3rd party version checker. We can add a header "Version Check URL:" The plugin doing the check can readily find *all* plugins that have this header and check those URLs for update info. I'm not sure how extensive plugin registration helps this process.

For a specific use case, look at this: ​http://code.google.com/p/strider-core/
This is a plugin framework I'm working on, that includes the aforementioned version checking. There is no download under the "main" code section, but look in Downloads for a sample plugin that uses the framework. (It makes more sense in context anyway....)

[strider72]

One more comment about making plugins "register" their own custom headers:

There are cases where you want the header to be returned even for plugins that are not activated. Again, the version check plugin is a good example. If a plugin has to register itself, this can't be done unless the plugin is actively running. What I'm trying to do is simply make it so any plugin can check plugins (itself or others) for additional specific headers.

Of course, if you want to check a header and only have a function do something to Plugin X, your function can be coded to only act if it's dealing with Plugin X... but that's up to the programmer.

[Denis-de-Bernardy]

broken patch

[Denis-de-Bernardy]

improved patch, updated against 11255

[Denis-de-Bernardy]

problem on keys

[Denis-de-Bernardy]

nm, the keys are overridden as needed by array_merge. with the patch I uploaded, you cannot override one of the default keys using a plugin -- i.e. the expected and needed behavior.

[westi]

Lets leave this to 2.9 for now as we are feature frozen and I want to make sure we get this feature right.

[Denis-de-Bernardy]

well, you might want this in 2.9 rather than 2.8, which I can understand if we're in feature freeze. but it's still tested and commit-worthy... dunno why you removed it from the commit candidate list.

[Denis-de-Bernardy]

it could easily make it in 2.8, too. I sort of need this for one of my own plugins, and I'm apparently not alone. currently, I end up re-opening every plugin file a second time to extract a single field. it has worked well for 3 WP versions, at it'll continue to work for another one, but that hardly is the most efficient way to do things.

imo, this should get into 2.8, since it works and you've at least two people here who are ready to nail down potential issues on the spot. and we'd enhance it afterwards (though I can't think of anything *else* we'd want in addition).

else, the ticket may end up just sitting there for years, as is so typical in trac.

[Denis-de-Bernardy]

See also #9532

[markjaquith]

+1 for early 2.9

[Denis-de-Bernardy]

amen to that, then.

[strider72]

When this is committed I'll do an equivalent patch for themes (using same code, basically). Both could then make it into 2.9.

[strider72]

Okay -- my original patch passed $plugin_file to the filter function, and the new patch doesn't.

Not sure if this is important or not, but I thought the distinction worth noting, as it was a sort of silent change between versions....

[Denis-de-Bernardy]

Picture a third party looking into adding headers that your plugin adds, without you knowing it.

[Denis-de-Bernardy]

still applies clean

[strider72]

Replying to Denis-de-Bernardy:

Picture a third party looking into adding headers that your plugin adds, without you knowing it.

This doesn't make sense (at least it's not a problem). So what if two plugins add the same header? Because the header is the array key, it's only added once, and because the returned parameter *is* the Header Name, they both add the same header that will return the same parameter within the plugin_data array. So it's no problem.

...unless your patch does something different than mine did. If you add "New Header", then

New Header: foo

gets us plugin_dataNew Header? = 'foo'

It doesn't matter which one, or how many, different plugins add "New Header". If it's added it's added for *all* plugins. (And as most plugins won't have that custom header, they'll just return "".)

[strider72]

Stoopid wiki formatting. Should be:

gets us plugin_data['New Header'] = 'foo'

[strider72]

Aha! Denis, your updated patch leaves out a *very* important line:

// Use keys to reduce conflicts and make conflicts easier to find.
foreach ( $extra_headers as $key=>$value ) { $extra_headers[$key] = $key; }

This goes just *before* the array_merge. What this does is (by design) completely ignores the value, and uses only the array keys. Thus you can't have the conflicts that you are concerned about. The key of the new parameter in plugin_data is *always* the exact same as the name of the header.

As I looks at that code, however, I see a way to make it better. Try this instead -- again just before the array_merge:

$extra_headers = array_unique( $extra_headers );
$extra_headers = array_combine ( $extra_headers, $extra_headers );

This is an improvement from the plugin author's standpoint. Rather than adding a key, e.g. $extra_headers['New Header'] = true; they just have to add a value: $extra_headers[] = 'New Header';

Array_unique removes duplicate values, and then the array_combine takes all the values and sets those as both Key and Value for each element. Perfect! :)

[strider72]

Correction: We don't need array_unique, because array_combine already effectively does that. (Not normally, but because we're in the unusual position where keys and values are the same, it works.)

So, just add this right before the array_merge line in Denis's patch:

$extra_headers = array_combine( $extra_headers, $extra_headers );

[strider72]

ARGH! Array_combine is PHP 5! Sorry, sorry, sorry. (And sorry for spamming the inboxes of whomever get updates on this patch.) We need this to work with PHP 4. This should do it:

$extra_headers = array_flip( $extra_headers );
foreach( $extra_headers as $key=>$value ) {
   $extra_headers[$key] = $key;
}

Again, sorry for so many posts. This should work out nicely. Add it to Denis's patch, just above the array_merge line.

[strider72]

With the new patch 8964, plugins can add new plugin headers by doing something like this in a hooked function:

$extra_headers[] = 'My New Header';
return $extra_headers;

[strider72]

"Plugin Header Test" plugin that demonstrates usage and tests against potential abuse. See source for notes.

[strider72]

Example of usage in a plugin:

function header_demo( $extra_headers ) {
	$extra_headers[] = 'Demo Header';
	return $extra_headers;
}
add_filter( 'plugin_headers', 'header_demo' );

[strider72]

Two files added:
1) New patch that updates for current code and solidifies security
2) Demo plugin

Demo plugin both adds headers and has notes in source showing what's happening.

Demo plugin also has faux "malicious" hook which demonstrates that attempts to alter default headers should fail; this patch *only* allows the addition of new headers.

[westi]

Ok had a look over this.

I'm wondering if the following changes might improve things:

• Rename the plugin_headers filter to extra_plugin_headers
• Expect the array back from extra_plugin_headers filter which contains Name => Regex match like the one specified for the normal headers.
• Use array_merge($extras,$defaults) so that you can't overwrite the defaults.

Otherwise all looks good :-)

[strider72]

1) Good idea, since it doesn't allow you to change the existing headers

2) Could you explain what you mean? The only thing that matters in the returned array is the values. Keys are irrelevant. In the final it uses $value => $value for the new header

3) It already does this:
$all_headers = array_merge($extra_headers, $default_headers);

[westi]

Replying to strider72:

1) Good idea, since it doesn't allow you to change the existing headers

2) Could you explain what you mean? The only thing that matters in the returned array is the values. Keys are irrelevant. In the final it uses $value => $value for the new header

Give extra headers the same ability as the built in ones - i.e. the key name that ends up in the array can be different from the string matched in the file.

3) It already does this:
$all_headers = array_merge($extra_headers, $default_headers);

True - I was refering to removing all the array_flippery which seemed like a bit over the top - just make the requirements on the plugin behaviour like I suggest about and then that can all be stripped out.

[strider72]

Give extra headers the same ability as the built in ones - i.e. the key name that ends up in the array can be different from the string matched in the file.

I explicitly avoided this so that different plugins can't accidentally overwrite each other.

What happens when one plugin adds a header of "Foo" with a key of "baz" and another adds a "Bar" header also with a key of "baz"? Can't have both. If the header and key (by definition) match, it simplifies things greatly. "Foo" is "Foo" and "Bar" is "Bar". Period.

[strider72]

Note: somewhere down the road when WordPress moves to PHP 5+, all the "array_flippery" can be reduced to one line:

$extra_headers = array_combine( $extra_headers, $extra_headers );

As I see it, the only change now is to change the "plugin_headers" hook to "extra_plugin_headers".

[strider72]

New patch changes "plugin_headers" hook to "extra_plugin_headers". Also some (very) minor code cleanup.

[strider72]

Better security than 8964.diff. Avoids plugins overwriting each other's headers by forcing key and value to be the same. Changes "plugin_headers" hook to "extra_plugin_headers"

[strider72]

Adds hooks to both plugins and themes, new function get_file_data()

[strider72]

New patch. I've abstracted the header-fetching to a separate function which is used by both plugins and themes. So, yes, this patch adds both an extra_plugin_headers hook and an extra_theme_headers hook.

Basically, you can make a wrapper function that:

1) defines an array of the headers to be searched for
2) passes it to get_file_data() along with the file
3) optionally, passes a $context string that automatically instantiates an extra_$context_headers hook.

This also allows plugins to use this function to search for similarly styled headers in different types of files. Makes it easy for a plugin to fetch data relating to custom config files or such that apply to itself. :)

[strider72]

bugfix -- patch 4 didn't properly handle Author header in all cases

[strider72]

Just added a patch that fixes a bug in the previous patch. No other changes, though it's nice to see it still applies clean. :)

Westi, could you please take a look at this again? I would really like this committed for 2.9. Not sure if something particular is holding it up.

This patch:

1) adds extra_plugin_headers hook so plugins can add to the list of headers that are checked for in get_plugin_data().

2) adds extra_theme_headers hook. Same as above, but for themes.

3) adds new function get_file_data() so that plugins can use a standard method to examine a particular file for plugin-like headers. Function can even be used to add an arbitrary extra_XXX_headers hook.

4) Both get_plugin_data() and get_theme_data() now use get_file_data() for their core functionality, removing some code repetition from core.

[strider72]

In patch 5, there's a minor error in the documentation/header for the get_file_data() function. We need to remove the line:

* @param bool $translate If the returned data should be translated

and insert the line:

* @param string $context If included, a filter hook "extra_<$context>_headers" will allow other code to alter the headers array

[strider72]

Finding bugs, gosh stinking darn it! (Pardon my language)

In get_theme_data() from patch 5,

$theme_data['Name'] = $title = wp_kses( $theme_data['Name'], $themes_allowed_tags );

should be

$theme_data['Name'] = $theme_data['Title'] = wp_kses( $theme_data['Name'], $themes_allowed_tags );

Changing the subject slightly:

Also in get_theme_data() from patch 5, at the end just before output I have the line:

unset( $theme_data['AuthorURI'] );

That's there because I was rather slavishly ensuring that the output of the function was unchanged. Considering that get_plugin_data() now returns AuthorURI, get_theme_data should do the same. Thus, that unset() should be removed.

I'll get a new patch up that addresses all of the above (plus my earlier comment).

NOTE: if get_theme_data returns AuthorURI, it follows that it should also return Author without turning it into a link (again, making it the same as get_plugin_data). That would require changes to the Themes admin page though, so I'll leave that for a separate ticket.

[strider72]

Same as patch 5 with bug fixes as discussed in above comments

[strider72]

New patch 6 includes everything from patch 5 except:

1. Fixed bug that missed outputting "Theme" in get_theme_data. Identical to "Name", but presumably for backward compatibility? I'm just matching the output of get_theme_data as it already exists.

2. get_theme_data now also outputs "AuthorURI". Just a side-effect of how the new function works -- previous patch explicitly removed it, but why bother? Also brings it closer to parity with get_plugin data. Happy side effect, I say. :)

[strider72]

Patch 6 also includes a fix to the doc header of the get_file_data function. The parameters are described correctly now.

[westi]

I've kicked the tires on the latest patch and all looks good.

Found one typo but nothing else.

We already had some theme metadata tests in the wordpress-tests and I have added some for plugin metadata too.

Going to leave this open for a little while in case any bugs crop up in further tests.

[westi]

(In [12044]) Allow for plugins to enhance the number of metadata fields captured from plugin and theme headers. See #8964 props strider72.

[westi]

(In [12048]) Ensure that Title is always set for plugin data as it was before. see #8964.

[strider72]

Westi -- I don't recall the specifics, but I'm pretty sure your last-minute "Title" addition is redundant.

A change in core (a while back) took away the Title attribute from that function, and I submitted a patch to re-add it. Somebody else implemented a different solution involving a wrapper function of some sort. I'll see if I can find the old patch & what happened with it....

[strider72]

Westi: My error -- you are correct.

I was thinking of this: ​http://core.trac.wordpress.org/ticket/7479

...but the Title does still need to be set, as you have done. No changes needed.

[strider72]

This was committed a month ago. Should have been closed as "fixed" I assume.

[hakre]

well it depends on the point of view. For a plugin to add headers it must already be active with this implementation.

For a plugin that can add headers by just writing them in their file, the implementation must be different. I opened a new ticket regarding an improvement on the overall domain: #13699