Incorrect usage of drupal_goto() [#2022229]

Here is a bug I've found when we were using Secure Pages, but it could potentially break when CAS Server is used with any module that calls hook_drupal_goto_alter().

Basically drupal_goto() should be used for internal links only, as per the documentation:

Sends the user to a different Drupal page.

This issues an on-site HTTP redirect.

However in cas_server drupal_goto() is being used to redirect the user to the external service. On my site this meant the https was being turned into http by securepages (correctly), which broke logins because the connecting service required https.

The fix is easy, just replace the drupal_goto() with header('Location: blah');

Comment	File	Size	Author
#1	cas-cas_server_drupal_goto-2022229.patch	517 bytes	Rob230
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Rob230 CreditAttribution: Rob230 commented 18 June 2013 at 11:40

Status:

Active

» Needs review

File	Size
cas-cas_server_drupal_goto-2022229.patch	517 bytes

This patch fixes it for cas_server.module.

I believe the same mistake is probably made in cas.module, but I don't use that part of the module so I haven't done anything for it.

Comment #2

18 June 2013 at 11:46

Status:

Needs review

» Needs work

The last submitted patch, cas-cas_server_drupal_goto-2022229.patch, failed testing.

Comment #3

Rob230 CreditAttribution: Rob230 commented 18 June 2013 at 14:02

Status:

Needs work

» Needs review

#1: cas-cas_server_drupal_goto-2022229.patch queued for re-testing.

Comment #4

bfroehle CreditAttribution: bfroehle commented 9 July 2013 at 16:43

Sorry, I'm a little confused here. drupal_goto can certainly be used for external pages. It sounds like securepages is being weirdly overzealous in this case --- perhaps you should report the bug to them?

Comment #5

Rob230 CreditAttribution: Rob230 commented 10 July 2013 at 09:18

Hmm, I originally thought this was a bug with securepages too, but I changed my mind when I saw that the manual said this for drupal_goto:

Sends the user to a different Drupal page.

This issues an on-site HTTP redirect.

The manual could be wrong or outdated though. drupal_goto can certainly be used for external URLs, I just wasn't sure whether it should (or why you would need it).

The code for drupal_goto seems geared towards Drupal paths. It changes the redirect to $_GET['destination'] if necessary and it lets other modules modify the path before it turns it into a full URL, neither of which would be desirable if you were trying to send them to an external page in my opinion.

But perhaps you are right and securepages should be detecting if the URL is external and ignoring it. #1962854: Do not alter external URL seems to do exactly that.