link to cron.php broken when clean urls are disabled
Bart Jansens - August 17, 2008 - 09:16
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | system.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
On the status report page, the link to cron.php doesn't work when clean urls are disabled. The link then points to something like http://localhost/~bart/drupal-7/?q=cron.php&cron_key=1bf726cd2d82b83ea03...
The same thing happens in the cron tests. It executes GET to http://localhost/~bart/drupal-7/?q=cron.php
The url is probably generated using l("cron.php") which happens to work when clean urls are enabled, but doesn't work when clean urls are disabled.
#1
Can reproduce. Using external = true seems to help a bit. Attached is a patch which fixes the problem and tests pass now, however, I'm not sure if this is the way to go, so a good review is necessary I think :)
#2
This bug is certainly confirmed (I haven't tried it with clean urls on), but it is certainly broken with clean urls off. I'll review the proposed patch a bit later.
#3
As an additional comment, the 'outside' cron link as it is currently coded does work properly with clean urls turned on.
#4
Tested patch, applies cleanly, and solves the problem of failing cron-tests when clean urls are disabled, and continuous to pass when clean urls are enabled.
I think this a good and clean solution, as the alternative is worse and because we're dealing with an outlier. The alternative being: determining in url() whether the call is to cron.php (or some other existing file being: update.php, install.php and perhaps xmlrpc.php), and automatically assign $external = TRUE. This unnecessarily adds strain to an important function, to make life easier for only a few instances where it would fail.
Besides, we already work around this 'bug' in url(), for example the link to 'update.php' on the module listing page is created by means of $base_url . '/update.php', without using url().
Therefore, I'm for this patch, in this form. As a result I set this to CNR.
#5
Patch in #1 did not work for sites in a subdirectory.
#6
patch in #5 fails to apply.
#7
simple re-roll (maybe was just a whitespace change). Fixes tests and the status page.
#8
Committed to CVS HEAD. Thanks.
#9
Automatically closed -- issue fixed for two weeks with no activity.