? .DS_Store
? .cache
? .git
? .project
? .settings
? 197266-2.patch
? empty
? file-save-data[1].patch
? file_203204_2.patch
? file_30520_4.patch
? file_test_cleanup.patch
? logs
? test.php
? upload-fix-access-check-226853-8-6.x-7.x.patch
? sites/all/modules
? sites/default/files
? sites/default/settings.php
? sites/default/test
Index: modules/upload/upload.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/upload/upload.module,v
retrieving revision 1.206
diff -u -p -r1.206 upload.module
--- modules/upload/upload.module	15 Sep 2008 09:28:50 -0000	1.206
+++ modules/upload/upload.module	18 Sep 2008 20:51:23 -0000
@@ -495,27 +495,26 @@ function _upload_form($node) {
     }
   }
 
-  if (user_access('upload files')) {
-    $limits = _upload_file_limits($user);
-    $form['new']['#weight'] = 10;
-    $form['new']['upload'] = array(
-      '#type' => 'file',
-      '#title' => t('Attach new file'),
-      '#size' => 40,
-      '#description' => ($limits['resolution'] ? t('Images are larger than %resolution will be resized. ', array('%resolution' => $limits['resolution'])) : '') . t('The maximum upload size is %filesize. Only files with the following extensions may be uploaded: %extensions. ', array('%extensions' => $limits['extensions'], '%filesize' => format_size($limits['file_size']))),
-    );
-    $form['new']['attach'] = array(
-      '#type' => 'submit',
-      '#value' => t('Attach'),
-      '#name' => 'attach',
-      '#ahah' => array(
-        'path' => 'upload/js',
-        'wrapper' => 'attach-wrapper',
-        'progress' => array('type' => 'bar', 'message' => t('Please wait...')),
-      ),
-      '#submit' => array('node_form_submit_build_node'),
-    );
-  }
+  $limits = _upload_file_limits($user);
+  $form['#access'] = user_access('upload files');
+  $form['new']['#weight'] = 10;
+  $form['new']['upload'] = array(
+    '#type' => 'file',
+    '#title' => t('Attach new file'),
+    '#size' => 40,
+    '#description' => ($limits['resolution'] ? t('Images larger than %resolution will be resized. ', array('%resolution' => $limits['resolution'])) : '') . t('The maximum upload size is %filesize. Only files with the following extensions may be uploaded: %extensions. ', array('%extensions' => $limits['extensions'], '%filesize' => format_size($limits['file_size']))),
+  );
+  $form['new']['attach'] = array(
+    '#type' => 'submit',
+    '#value' => t('Attach'),
+    '#name' => 'attach',
+    '#ahah' => array(
+      'path' => 'upload/js',
+      'wrapper' => 'attach-wrapper',
+      'progress' => array('type' => 'bar', 'message' => t('Please wait...')),
+    ),
+    '#submit' => array('node_form_submit_build_node'),
+  );
 
   // This value is used in upload_js().
   $form['current']['vid'] = array('#type' => 'hidden', '#value' => isset($node->vid) ? $node->vid : 0);
Index: modules/upload/upload.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/upload/upload.test,v
retrieving revision 1.4
diff -u -p -r1.4 upload.test
--- modules/upload/upload.test	15 Sep 2008 09:28:50 -0000	1.4
+++ modules/upload/upload.test	18 Sep 2008 20:51:23 -0000
@@ -201,3 +201,156 @@ class UploadTestCase extends DrupalWebTe
     return NULL;
   }
 }
+
+class TestCase226853 extends DrupalTestCase {
+  function get_info() {
+    return array(
+      'name' => t('[226853] upload.module hard-codes \'upload files\' permission check'),
+      'desc' => t('The short version: _upload_form() should be using a form #access property to set its requirement for upload files permission, and not hard-code it in the module logic. Because it does not do this, the only way to extend this form from a contributed module is to hack core.'),
+      'group' => t('Drupal 7 Tests'),
+    );
+  }
+
+  function testIssue() {
+
+    $this->drupalModuleEnable('upload');
+
+    // A module makes new own permission and implements hook_form_alter()
+    $module_name = $this->randomName();
+    $module_text = <<< EOF
+<?php
+function {$module_name}_perm() {
+  return array(
+    'upload files to {$module_name}' => t('Attach files to content to something {$module_name} related.'),
+  );
+}
+
+function {$module_name}_form_alter(&\$form, \$form_state, \$form_id) {
+  if (\$form_id == '{$module_name}_form') {
+    \$obj->files = array();
+    \$form['attachments'] = array(
+      '#type' => 'fieldset',
+      '#title' => t('File attachments'),
+      '#access' => user_access('upload files to {$module_name}') || user_access('upload files'),
+      '#collapsible' => TRUE,
+      '#collapsed' => empty(\$obj->files),
+      '#description' => t('Changes made to the attachments are not permanent until you save this post. The first "listed" file will be included in RSS feeds.'),
+      '#prefix' => '<div class="attachments">',
+      '#suffix' => '</div>',
+      '#weight' => 10,
+    );
+    // Wrapper for fieldset contents (used by ahah.js).
+    \$form['attachments']['wrapper'] = array(
+      '#prefix' => '<div id="attach-wrapper">',
+      '#suffix' => '</div>',
+    );
+    \$form['attachments']['wrapper'] += _upload_form(\$obj);
+    if (isset(\$form['attachments']['wrapper']['#access'])) {
+      \$form['attachments']['wrapper']['#access'] |= user_access('upload files to {$module_name}');
+    }
+    \$form['#attributes']['enctype'] = 'multipart/form-data';
+  }
+}
+
+EOF;
+
+    $this->createModule($module_name, $module_text);
+
+    // check for a user has only 'upload files' permission
+    $this->checkPowerfulRole($module_name);
+
+    // check for a user has only module's permission
+    $this->checkModuleRole($module_name);
+
+    // check for a user has other permission
+    $this->checkRandomRole($module_name);
+
+    $this->deleteModule($module_name);
+  }
+
+  function createModule($module_name, $module_text, $module_core = '7.x') {
+    $module_dir = "sites/all/modules/$module_name";
+    $module_file = "$module_dir/$module_name.module";
+    $module_file_info = "$module_dir/$module_name.info";
+    if (file_check_directory($module_dir, FILE_CREATE_DIRECTORY)) {
+      $info = <<< EOF
+name = "$module_name module"
+description = "$module_name module for test. Delete it when you see it."
+package = Temporary
+core = $module_core
+
+EOF;
+      if ($fp = @fopen($module_file_info, 'w')) {
+        fwrite($fp, $info);
+        fclose($fp);
+      }
+      if ($fp = @fopen($module_file, 'w')) {
+        fwrite($fp, $module_text);
+        fclose($fp);
+      }
+      $this->drupalModuleEnable($module_name);
+    }
+  }
+
+  function deleteModule($module_name) {
+    $module_dir = "sites/all/modules/$module_name";
+    $module_file = "$module_dir/$module_name.module";
+    $module_file_info = "$module_dir/$module_name.info";
+    @unlink($module_file);
+    @unlink($module_file_info);
+    @rmdir($module_dir);
+  }
+
+  // Check 'upload files' permission.
+  // Should pass so 'upload files' is very general
+  function checkPowerfulRole($module_name) {
+    // function _upload_form() uses a global $user
+    global $user;
+    $orig_user = $user;
+    $form = array();
+    // create a user for form's preparing
+    $user = $this->drupalCreateUserRolePerm(array("upload files"));
+    $this->drupalLoginUser($user);
+    // prepare the form (executing any hook_form_alter)
+    drupal_prepare_form("{$module_name}_form", $form, $form_state);
+    $this->assertTrue($this->checkAccess($form), "check generic 'upload files'");
+    $this->drupalGet('logout');
+    $user = $orig_user; // restore the original user
+  }
+
+  // Check permission for upload by module's permission.
+  // Should pass so module's permission checked in hook_form_alter()
+  // It checks the issue: the user don't have 'upload files' permission,
+  // but he have module's permission though.
+  function checkModuleRole($module_name) {
+    global $user;
+    $orig_user = $user;
+    $form = array();
+    $user = $this->drupalCreateUserRolePerm(array("upload files to {$module_name}"));
+    $this->drupalLoginUser($user);
+    drupal_prepare_form("{$module_name}_form", $form, $form_state);
+    $this->assertTrue($this->checkAccess($form), "check module's 'upload files to {$module_name}'");
+    $this->drupalGet('logout');
+    $user = $orig_user;
+  }
+
+  // Check random permission
+  // Should pass (assertFalse) so random permission does not allow file upload
+  function checkRandomRole($module_name) {
+    global $user;
+    $orig_user = $user;
+    $form = array();
+    $user = $this->drupalCreateUserRolePerm(array($this->randomName()));
+    $this->drupalLoginUser($user);
+    drupal_prepare_form("{$module_name}_form", $form, $form_state);
+    $this->assertFalse($this->checkAccess($form), "Check other roles");
+    $this->drupalGet('logout');
+    $user = $orig_user;
+  }
+
+  function checkAccess($form) {
+    return !empty($form['attachments']['#access']) &&  // access to attachments
+           isset($form['attachments']['wrapper']['new']); // is 'new' form exists?
+           !empty($form['attachments']['wrapper']['#access']); // access to wrapper
+  }
+}