hey Larry, good bumping into you at DCon the other day... looking at the D6 security issue and don't quite get it yet; plus looking at your code for D7 i don't see how it is fixing greggels concerns either.

but.. more to this issue..

took a quick look at your D7 code and was wondering why on the admin page for autosave you check if jquery autosave plugin exists?

  if (!file_exists(AUTOSAVE_PATH . '/jquery.autosave.js')) {
    drupal_set_message(t('Unable to find the jQuery Autosave Plugin in !path. Please <a href="http://plugins.jquery.com/files/jquery.autosave_0.zip">download jquery.autosave.js</a>
      and place it into !path.', array('!path' => AUTOSAVE_PATH)), 'error');
  }

i think you supply the plugin with the code (which is typical use case) so why would you check if the file is there; surely we don't have to start writing checks for every file that makes up a module.

also, if for some reason you needed to be extra careful that someone hadn't only 1/2 untarred the module file; you only check for this on the admin page; which isn't where the plugin is used and which may actually never be hit if the user is ok with the defaults (which typically they would be).

i'll take another look at security issue later this weekend and maybe try to grab greggels on irc

CommentFileSizeAuthor
#1 autosave_remove_js_check.patch1.15 KBquicksketch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quicksketch’s picture

Title: why check for autosave jq plugin? » Remove check for autosave jQuery plugin because it's bundled anyway
Priority: Normal » Minor
Status: Active » Fixed
FileSize
1.15 KB

I'm helping Larry clean up the D7 version of the module. I agree there's absolutely no reason to include this check, since the file is bundled with the module. Additionally the link in the alert is now dead anyway, and the t() string was poorly formatted. Overall just an unnecessary chunk of code. I've removed it from both D6 and D7 branches.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.