Hi,

I note the request test function has changed so that instead of checking admin/reports/request-test it checks the home page:
http://api.drupal.org/api/function/system_check_http_request/6

This is a terrible thing for Single (or Shared) Sign-On users. The reason is that we need to explicitly exclude the request test URI from the Single Sign-On redirection behaviour in order for it to function correctly. We can do this from the admin UI of Single Sign-On, so it wasn't a big deal, but now we need to exclude the HOME PAGE, which is bad and is a big deal. It basically means the home page of any Drupal website using Single Sign-On now can't be included in the check (e.g. when someone visits the home page of one site from another site in the same Single Sign-On group, they *won't* be automatically logged in).

This is critical for us (and I'm sure there will be many others affected, even if they haven't noticed yet).

Please change this back! AFAIK, it wasn't broken, so I'm not sure why someone broke it...? =(

Comments

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

Ps - someone told me on Twitter that there may have been some cases where the old behaviour was causing problems too... sounds like there needs to be an admin option here (e.g. Use home page or Use request test page as a radio), since there isn't one size that fits all...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 6.9 » 7.x-dev

CVS annotate shows that this change was introduced in #245990: HTTP request testing unreliable and may be undesirable.

So your fix for this issue needs to not re-introduce the issues all of the people in that thread were experiencing. Your fix should come with tests, so we can ensure that things are working as expected. It needs to be fixed in 7.x first, and then back-ported.

If you need help with any of this, drop by #drupal on irc.freenode.net and we can help you get a development environment set up.

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented
Assigned: Unassigned » greg.harvey

Thanks for the info. Never made any changes to core before, so I'll read up before I start. Will hopefully get chance to look at this Wednesday/Thursday this week...

David_Rothstein’s picture

David_Rothstein commented

The reason is that we need to explicitly exclude the request test URI from the Single Sign-On redirection behaviour in order for it to function correctly.

I've having a little trouble understanding what breaks in this case. In other words, if you were to not exclude this URI from the Single Sign-On redirection behavior, what exactly would happen?

Also, if you need to completely disable the HTTP request test on your site, a workaround for that is to put this in your settings.php file:

$conf['drupal_http_request_fails'] = FALSE;
greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

Hi David,

Thanks for the reply - basically, if you don't exclude it then Single Sign-On tries to redirect Drupal to the selected "master server" to check for an existing session or register a new session. This redirect behaviour prevents Drupal from seeing what it expects (the "request test" string) because it doesn't seem to follow the redirect through for some reason? Probably something the Single Sign-On module does. As a result, the request test fails. But we can get around this by excluding that URI from being checked.

Anyway, that aside the config change will fix our problem. I don't think we need anything else. Not sure where to close this or leave it open. I'll leave it to you, but we are happy to use your above suggested line in the config file. =)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Change in request test behaviour breaks all Single Sign-On websites that aggregate other websites over HTTP » Single Sign-On can't deal with HTTP request test
Project: Drupal core » Single-Sign-On Multi-Database
Version: 7.x-dev »
Component: system.module » Code

I'll move this to Single Sign-On module for now. Seems like we're covered in core.

David_Rothstein’s picture

David_Rothstein commented
Priority: Critical » Normal

Thanks for the explanation, Greg... Hmm, this seems surprising, since the specific problem you're describing is exactly the sort of thing that the fix which went into Drupal 6.9 was designed to prevent!

In particular, in Drupal 6.8 and earlier, the HTTP request test was searching for particular content on the page, but you can see from http://api.drupal.org/api/function/system_check_http_request/6 that it is now actually only looking for a valid HTTP response. So even if it gets back a 301 from the redirect, the check should still pass. It would be interesting to track down why it is failing in your case... I wonder if the problem is that it is following the redirect, but somewhere down the line it is reaching something where the request fails?

Also, I'm lowering the priority down to normal, because even without putting that line in the settings.php file, I'm pretty sure the bug won't actually be too bad -- this was one of the other aspects of the change that went into Drupal 6.9. Previously, a failed HTTP request check meant that Drupal would shut off its own ability to make an actual HTTP requests (and that was indeed bad), but as of Drupal 6.9 it doesn't do that anymore. So, theoretically at least, the only bug here is that an erroneous red message will be appearing on the admin screen.

I'm guessing that whatever fix eventually gets written at #245990: HTTP request testing unreliable and may be undesirable will have to take care of this situation too... Since it appears there are still a variety of cases where the new request test doesn't work, I was thinking over there that there might have to be a backup check (after the local request test fails) that tries to reach an external website, and only if that fails too would it give up and show the red error message...

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

Hi David,

Thanks for the additional background. I'm in the office tomorrow and will talk to the developer who discovered the issue and see if I can gather more detail. All I know thus far is we are getting watchdog errors telling us the request test is failing and, judging by the context, Single Sign-On is the point of failure. I'll devote some time tomorrow to understanding *exactly* what is going on and report back.

Also you're quite right to down-grade the severity - I didn't realise the consequences of a failure had changed, so it's no longer as critical if a single request test fails. Previously a request test fail on one of our web services caused Drupal to run the request test, which also failed, killing everything! Since this is no longer the behaviour, this is a much less serious issue. Good stuff all round... =)

greg.harvey’s picture

greg.harvey
Primary language English
Location Occitanie, France
commented

Hi David,

Coming back on this, I have been trying to make request tests fail on our dev server and I can't do it. Not sure where these messages are coming from. I'm starting to suspect the request test variable was already set to false when the core was still Drupal 6.8, no one reset it (nor did the update) so some sites still reported they could not make request tests. Will look in to this, but in short, I can't replicate the problem now on our dev server.