I am using a login modal on a page where users must sign in to post comments. The link to login looks like this: /modal_forms/nojs/login?destination=node/4093%23comment-form

The modal opens properly, and the user is able to sign in, however the redirect results in an error. The culprit is that the hash sign is escaped as '%23'. How can I prevent this from happening? The link should look like this: /modal_forms/nojs/login?destination=node/4093#comment-form.

For now, I made the following change to modal_forms.pages.inc:

diff --git a/modal_forms.pages.inc b/modal_forms.pages.inc
index b9b3817..5770d77 100644
--- a/modal_forms.pages.inc
+++ b/modal_forms.pages.inc
@@ -36,7 +36,8 @@ function modal_forms_login($js = NULL) {
     ctools_add_js('ajax-responder');
     $output[] = ctools_modal_command_dismiss(t('Login success'));
     if (isset($_GET['destination'])) {
-      $output[] = ctools_ajax_command_redirect($_GET['destination']);
+	  $url = explode("#", $_GET['destination']);
+	  $output[] = ctools_ajax_command_redirect($url[0]);
     }
     elseif (module_exists('login_destination')) {
       $destination = login_destination_get_destination('login');

This just removes the hash sign and everything that follows it.

Can anyone think of a better solution?

Comments

a.milkovsky’s picture

Version: 7.x-1.2 » 7.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new3.01 KB

Here you are. A better destination handling using all available url options including get parameters and # fragment.

a.milkovsky’s picture

unfortunately if destination points to the same redirect does not happen, temporary fix using:
$options['query']['modal_forms_redirect'] = TRUE;

a.milkovsky’s picture

Title: Hash sign (#) escaped in url redirect causing error » Special signs (? or #) escaped in url redirect causing error
Priority: Normal » Major

Any update on this issue?

fago’s picture

Status: Needs review » Reviewed & tested by the community

So this is fixing two things, redirecting back to the origin and having urls with ?# characters in the destination parameter. I tested the fix and it works fine. The modal_forms_redirect trick is not very elegant, but works fine - so why not.

Related, I was wondering whether this code allows for an open-redirect attack - but in my tests it worked fine as it stripped the external host (before and after the patch). So RTBC.

n.kishorekumar’s picture

Faced same problem, after applying this patch its worked fine,

Thanks for the patch