Question about httpbl_boot()
| Project: | http:BL |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | praseodym |
| Status: | needs work |
Jump to:
(This is a "possible bug" but they don't have a category for that.)
I'm looking at the 2009/09/12 commit of httpbl and I see there have been changes made to httpbl_boot() .
I'm particularly wondering about:
if ($_GET['q'] == 'httpbl/whitelist') {
return;
}I've had my eyeballs buried in bootstrap documentation for most of the day, related to the separate issue of #570742: Logging is not working. Something I have read there may impact the code above, just as it has had an impact on logging.
Specifically, $_GET['q'] does not get set until the eighth phase of the bootstrap. (http://api.drupal.org/api/constant/DRUPAL_BOOTSTRAP_PATH/6)
When I was trying to hack my way through the logging problem in httpbl_boot(), I learned that at the point that httpbl_boot() is invoked, the bootstrap process is only at phase 6.
So what I'm getting at is, as seems to be the case with logging, you may also need a call to the full bootstrap in httpbl_boot() to ensure that $_GET['q'] is set before testing it here. If not the full bootstrap, at least to phase 8, and that seems to be only one phase short of FULL.
Other than that, I'll be the first to admit I don't really understand what that test is about to begin with, or why it is there.
#1
You're right. The test is there so that we don't block requests to the whitelist-page (greylisted users can request to be whitelisted), rendering the whitelisting system useless. I'll need to figure something out to test this without a full bootstrap.
#2
See #24 in #570742: Logging is not working. It (of course) has to do with bootstrapping for the logging to work, but doing it in a place where it only happens when needed, after a positive hit on grey or blacklisted visitors. I think that possibly also addresses this because I was able to test the whitelisting page with no problems.
#3
Similar to the logging problems in #570742: Logging is not working, the line ...
if ($_GET['q'] == 'httpbl/whitelist') {return;
}
... produces an "undefined index:q" PHP notice condition. The similarity to the logging problem is that is happens because we are not yet through enough of the bootstrap process to successfully execute that code with any "Drupal meaning."
However, that code never gets executed unless there is a Grey-list condition. If the logging is fixed, using the patch in #25 of #570742: Logging is not working, then this code will work without any additional changes.
#4
I haven't put together an updated patch for this yet, but upon further thought I realize the above solution needs to be refined a bit.
The
if ($_GET['q'] == 'httpbl/whitelist')code is only executed when we have a potential greylist candidate for a whitelist challenge. The bootstrapping completion required for logging would also take care of this code not working, but in the patch above the bootstrapping is also only happening under the condition of the Verbose logging option being TRUE.In order to kill both of these birds with one stone, the bootstrapping in a greylist condition needs to happen whether or not logging has been opted for.
#5
The problem with this early bootstrapping is that I'm not sure if it has any (potential) side effects for Drupal itself or other modules. Other than that there's also the negative effect on performance which we're actually trying to reduce. With the early bootstrapping, a 'malicious' visit to a cached page is actually worse performance-wise than a regular visit to the same page, since the 'malicious' visit requires a full bootstrap while the regular visit will simply be served by the cache stage.
#6
I think we've been through this. My reading of the API is that this is not really a matter of of "early bootstrapping" so much as simply allowing any bootstrapping already in progress -- and only about one or two phases short of completion -- to complete. I'm also not clear of what side effects you are concerned about avoiding. Can you name any specifics? I believe we're only referring to bootstrapping within the confines of a unique visit session, not the entire operation of Drupal being used by other visitors. Also, as I've proposed it, it only happens once in the checking process, when a visit has been caught in the trap, at which time it will then be placed into cache. It is not as though this bootstrapping will be a repeating occurrence for each attempted subsequent visit from the same IP.
Again, it will only happen once, when a visit comes from a new IP, not already in {httpbl}, that fits the grey or black-list criteria based on the Honeypot threat level. In the case of a greylist, I think you're already essentially going to have the fully bootstrapped (themed) condition once they reach the whitelist challenge form, but, as written, you'll need that condition just to get to that page.
Other than that, can you suggest an alternative approach?