Problem/Motivation

The code in securepages_drupal_goto_alter() should leave unchanged a path related to an external URL. It seems clear that the matching path and role redirect rules apply only to the current site (with the running code) and that the current site has no basis to change the path on another site.

Proposed resolution

See patch attached to first comment, which includes a test.

Remaining tasks

Nothing.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#9 securepages-leave_external_url-1962854-9.patch1.57 KBmrmikedewolf
#1 1962854-leave-external-url.patch2.1 KBsolotandem

Comments

solotandem’s picture

solotandem commented
Assigned: Unassigned » solotandem
Status: Active » Needs review
StatusFileSize
new 1962854-leave-external-url.patch2.1 KB

Attached patch implements proposed resolution.

rob230’s picture

rob230 commented

Secure Pages breaks a number of my sites that have another module using drupal_goto() to send the user to an external page.

The simple change of returning if the URL is external is a good solution.

dan_lennox’s picture

dan_lennox commented

I've tested this patch on a production site and it's working great.

dan_lennox’s picture

dan_lennox commented

I've tested this patch on a production site and it's working great.

dan_lennox’s picture

dan_lennox commented

I've tested this patch on a production site and it's working great.

dan_lennox’s picture

dan_lennox commented

I've tested this patch on a production site and it's working great.

dan_lennox’s picture

dan_lennox commented

Apologies for the duplicate comments... on submission it's posted 4 comments and caused an nginx error...

merzikain’s picture

merzikain commented
Issue summary: View changes

The patch is working for me on a production website.

mrmikedewolf’s picture

mrmikedewolf commented
StatusFileSize
new securepages-leave_external_url-1962854-9.patch1.57 KB

Great patch! Just a small update. I removed hunk 1 of the modifications to salesforce.test as this change has been rolled into the newest version of the module.

rob230’s picture

rob230 commented

Sorry, can you explain what you have changed and what salesforce.test is?

From what I can see you've removed the call to _testGoto() but not removed the function itself. And there doesn't seem to be an equivalent test for external paths in the latest dev version so I'm not sure why it's removed.

steven jones’s picture

steven jones commented

Great patch in #1! Just quick review:

+++ b/securepages.test
@@ -345,6 +346,28 @@ class SecurePagesTestCase extends DrupalWebTestCase {
+    $is_https = TRUE;

Can we be sure that $is_https was TRUE at the start of the test?
Or should we revert this to the previous value, whatever that is?

Also, I arrived at this issue because I have a full path going through drupal_alter, i.e. one that already has https at the front, but is a URL for the current Drupal site, so it's handled as an external URL, and secure pages changes it to a http URL :(

astonvictor’s picture

astonvictor commented
Status: Needs review » Closed (outdated)

I'm closing it because the issue was created a long time ago without any further steps.

if you still need it then raise a new one.
thanks