Patch to follow.

CommentFileSizeAuthor
#11 allow_cron-2030017-11.patch1.6 KBbalsama
#6 allow_cron-2030017-6.patch1.6 KBbalsama
#5 allow_cron-2030017-6.patch1.27 KBbalsama
#1 cron_fails-2030017-1.patch440 bytesbalsama
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

balsama’s picture

balsama
he/him
Primary language English
Location boston
CreditAttribution: balsama commented
Status: Active » Needs review
FileSize
cron_fails-2030017-1.patch440 bytes

Patch attached.

balsama’s picture

balsama
he/him
Primary language English
Location boston
CreditAttribution: balsama commented

Since the D7 module does allow you to exclude paths, an alternate approach might be to set the default value of `shield_excluded_paths` variable to cron.php. Either way, I think something should be done because it's not obvious that enabling and configuring this module will break functionality.

If the maintainer prefers the `shield_excluded_paths` approach, I can provide a patch for that as well.

kalman.hosszu’s picture

kalman.hosszu CreditAttribution: kalman.hosszu commented

@balsama

Thank you for your patch but I don't think that is a good idea to hardcode a bypass. The exclude idea is good but the problem is that the cron.php is "out of Drupal" so it's not available in GET['q']. However we could append the admin interface with a checkbox to enable run cron from browser.

What do you think?

balsama’s picture

balsama
he/him
Primary language English
Location boston
CreditAttribution: balsama commented

Right. It would require rewriting the logic in the exclude path functionality. I'll provide another patch with the same functionality as my previous, but with a checkbox like you already have for drush.

I really can't think of a scenario where you wouldn't want to bypass cron and drush, but that's probably a separate issue (and may have already been considered/discussed).

balsama’s picture

balsama
he/him
Primary language English
Location boston
CreditAttribution: balsama commented
FileSize
allow_cron-2030017-6.patch1.27 KB

Patch attached.

balsama’s picture

balsama
he/him
Primary language English
Location boston
CreditAttribution: balsama commented
Status: Needs work » Needs review
FileSize
allow_cron-2030017-6.patch1.6 KB

Rerolled the patch against the latest dev version. This patch also includes deleting the cron variable on uninstall which was missing from the previous.

kalman.hosszu’s picture

kalman.hosszu CreditAttribution: kalman.hosszu commented 
+++ b/shield.admin.incundefined
@@ -26,6 +26,13 @@ function shield_admin_settings($form, &$form_state) {
+  $form['general']['shield_allow_cron'] = array(

+++ b/shield.moduleundefined
@@ -47,6 +47,11 @@ function shield_boot() {
+  if (basename($_SERVER['PHP_SELF']) == 'cron.php' && variable_get('shield_allow_cron', 1) == 1) {

The created variable should delete during the uninstall process.

EDIT: sorry i wrote it before you sent the upgraded patch.

kalman.hosszu’s picture

kalman.hosszu CreditAttribution: kalman.hosszu commented
Status: Needs review » Needs work

When I try to apply your patch I get the following error message:

▶ git apply -v allow_cron-2030017-6_0.patch
Checking patch shield.admin.inc...
error: while searching for:
    '#default_value' => variable_get('shield_allow_cli', 1),
  );

  $form['general']['shield_excluded_paths'] = array(
    '#type' => 'textarea',
    '#title' => t('Exclude these paths from Shield protection'),

error: patch failed: shield.admin.inc:26
error: shield.admin.inc: patch does not apply
Checking patch shield.install...
error: while searching for:
 */
function shield_uninstall() {
  variable_del('shield_allow_cli');
  variable_del('shield_user');
  variable_del('shield_pass');
  variable_del('shield_print');

error: patch failed: shield.install:9
error: shield.install: patch does not apply
Checking patch shield.module...
error: while searching for:
    return;
  }

  // Allow excluded paths to bypass Shield protection.
  $excluded_paths = variable_get('shield_excluded_paths', '');
  if (!empty($excluded_paths)) {

error: patch failed: shield.module:47
error: shield.module: patch does not apply

Is 7.x-1.x the repository that you use and it is up-to-date?

balsama’s picture

balsama
he/him
Primary language English
Location boston
CreditAttribution: balsama commented

It applies cleanly for me. Sounds like you might have tried to apply it twice or you made a different change to the .install file.

I'm definitely running off the latest 7.x-1.x version:

$ git reset --hard
HEAD is now at e2760af issue #1829694 by Boobaa: FastCGI and plain old mod_cgi support.
$ git apply -v allow_cron-2030017-6_0.patch
Checking patch shield.admin.inc...
Checking patch shield.install...
Checking patch shield.module...
Applied patch shield.admin.inc cleanly.
Applied patch shield.install cleanly.
Applied patch shield.module cleanly.
kalman.hosszu’s picture

kalman.hosszu CreditAttribution: kalman.hosszu commented

Would you pull the branch?

▶ git reset --hard
HEAD is now at 434aeb3 Issue #1898216 by 7wonders: Added Choose shield method = protect all, exclude or include.

It seems that your git HEAD is not up-to-date.

balsama’s picture

balsama
he/him
Primary language English
Location boston
CreditAttribution: balsama commented
FileSize
allow_cron-2030017-11.patch1.6 KB

Aha! Here we go. Updated patch attached. Applies cleanly to 434aeb3.

kalman.hosszu’s picture

kalman.hosszu CreditAttribution: kalman.hosszu commented
Category: bug » feature
Status: Needs review » Fixed

Thanks for the work, I have just committed it: http://drupalcode.org/project/shield.git/commit/4ff6131

kalman.hosszu’s picture

kalman.hosszu CreditAttribution: kalman.hosszu commented
Title: Port 2029997 (allow cron to run) to 7.x version » Allow cron to run

Status: Fixed » Closed (fixed)

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