jeremyclark13 / automatic-theme-plugin-update

Self hosted plugin and theme update scripts
http://clark-technet.com/2010/12/wordpress-self-hosted-plugin-update-api
548 stars 147 forks source link

Improvements #17

Open turtlepod opened 11 years ago

turtlepod commented 11 years ago

just some suggestion, i'm not on my computer right now, so this is only some that i remember.

API Script

index.php on basic_check we don't need to feed all the data to wp. just "slug", "version" and "package" is needed in transient. i'm not even sure "slug" is needed (it's for plugins i can't remember what's the req for theme)

on plugin_information it's better to set external to true. and not all html is allowed in sections. i''ll posted later list of allowed html in sections data. so this need to be sanitized and validate before sending or after recieving the data (or both, just to make sure)

packages.php theme screenshot is not used, it sill use screenshot from the themes (so it's not needed)

in plugin sections tab, you did it wrong, we cannot create our own tab, the allowed tab are:

everything need to be simplified, you don't need multiple array in there when you array_shift-ed the package in index.php.

something like this might be better:

$packages['slug'] = array(
    '0.1.0' => array(
        'author' => '',
        //other data
        //version in not needed, already in array data
        //external is not needed, add it in index.php before sending
        //need better way to handle sections to easy section input, try file_get_contents or similar solutions.
    );
);

Plugin example

$request_string: don't need api-key, i don't think we use it anywhere. unserialize: may be better to use maybeunserialize wp functions (it's better) bad global naming. not all plugin file name is the same as plugin folder name.

i'm not really sure, but in

$checked_data->response[$plugin_slug .'/'. $plugin_slug .'.php'] = $response;

there's a possibility that response is not set yet (?) since it's transient so it's better to set it first. when handling transient data (cause it's temporary data), it's better to assume that the data is not always available.

i guess that's it for today! i'll try to read it more in depth once i have the time.

jeremyclark13 commented 11 years ago

Thanks so much for these suggestions. Most of the code was taken from another package and I just kept it up to date with wp versions. I do believe at one time you could set an arbitrary section title. I haven't tested in the latest version. Also I think the screenshot url for themes was used for wpmu notifications st one time. I'll have to double check this again though.

Sanitizing the html befor display is a very good idea as well. Didn't think about the consequences if the api server is compromised it could send malware thru the plugin information screen.

Thanks again. I'll try to do a more in depth review of the WordPress update system myself.