The cancel link on the approve/disapprove/cancel form does not use the destination query if it is set. Since the form is normally loaded via AJAX it's not a major defect but if the form is called from a different location and AJAX is not used the cancel link needs to redirect correctly.

See attached patch for proposed fix.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alex.k’s picture

Thanks but don't you think someone could craft a URL that requests to approve a relationship and insert a destination to a phishing site?

elliotttf’s picture

If that were true wouldn't the form redirect to the phishing site upon submission? I don't know if the Forms API sanitizes destinations in any way, but I thought it redirected to whatever the destination query is set to.

alex.k’s picture

Yep it will redirect to whatever is given, hence my concern. So it seems risky to include it in UR due to the potential of misuse.

elliotttf’s picture

Right, but the form API will redirect all forms to a given destination. This only becomes malicious if

  1. the site is compromised and destination queries are altered, or
  2. a malicious user creates a URL with an undesirable destination

In the first case you've got bigger problems to worry about than form redirection and in the second case site admins can remove the links/posts/users that introduce the vulnerability. I guess I'm just not clear why it's ok to redirect a form submission to destination but not the form cancel link.

alex.k’s picture

Category: bug » feature

The #2 is what I was thinking about... but you're right, submit and cancel should both respect destination if one does so. (for that matter, it would have been better to use a submit for cancel, not a link, perhaps I should modify it this way)

elliotttf’s picture

Personally I like the link because you don't have to write any submit handler code for it, but as long as there's an option to abort the submission that returns to the same place that submitting does I'm happy.

mErilainen’s picture

subscribing

I'm not sure what this "destination query" is, but currently users are always redirected to relationships/requests whether they approve, deny, cancel or say no in the AJAX form. Only when I add a relationship from user's profile, then it stays on the same page and prints a message that my request has been sent.

YK85’s picture

Priority: Minor » Normal

Hello,

Is there a chance of this patch being committed? or will there be further development?
I would like to set the URL to a specific page when users click cancel from the approve/remove page, but it currently leads to relationships/requests

Thank you!

robby.smith’s picture

+1 subscribing

Bilmar’s picture

Hello,

I am getting the below when trying to test the patch:

can't find file to patch at input line 8
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: user_relationships_ui.forms.inc
|===================================================================
|RCS file: /cvs/drupal-contrib/contributions/modules/user_relationships/user_relationships_ui/Attic/user_relationships_ui.forms.inc,v
|retrieving revision 1.1.2.12
|diff -u -p -r1.1.2.12 user_relationships_ui.forms.inc
|--- user_relationships_ui.forms.inc	14 May 2009 09:53:11 -0000	1.1.2.12
|+++ user_relationships_ui.forms.inc	12 Jun 2009 16:18:45 -0000
Bilmar’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
790 bytes

I'm re-rolling the original patch so it applies smoothly.
Tested and it works great! Thanks for the work elliotttf

YK85’s picture

tested this and it fixed the destination query issue
+1 for commit

chuckbar77’s picture

Alex - could you please check this out and commit? It helps greatly to fix the destination query issue for customized sites.

alex.k’s picture

Status: Reviewed & tested by the community » Fixed

Committed in http://drupal.org/cvs?commit=420344, thanks all.

Bilmar’s picture

thanks Alex the wave of commits!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

mdupont’s picture

Title: Approve, Disapprove, Cancel Form Should Respect Destination Query » Relationship action forms should respect destination query
Category: feature » bug
Status: Closed (fixed) » Needs review
FileSize
1.05 KB

Sorry to reopen but the previous fix only covers Approve, Disapprove and Cancel actions. Destination query should also be respected in Remove action, and (but I'm less sure of that) Request action, thus all actions will act in a consistent way.

It proves useful when using UR actions from somewhere else than the standard pages (in views for example) and no ajax is used.

Attached a patch that make the Remove action respect destination query. Should it be the same for Request action? What do you think?

mdupont’s picture

After looking more closely at the code I think Request action should respect destination query too. Patch attached.

mrf’s picture

Issue tags: +6.x-1.1

Tagging to remind myself to take a look later.

mrf’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Patch (to be ported)

Committed to 6.x-1.x. Looks like this portion is missing from 7.x as well.

Berdir’s picture

Status: Patch (to be ported) » Fixed

Yep. Commited.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

srees’s picture

Version: 7.x-1.x-dev » 6.x-1.3
Status: Closed (fixed) » Needs review

The destination setting does not work for the cancel button with URL's that contain encodable characters. For example, if you set destination=user/somepage?var=1, the submit works correctly, but the cancel link takes you to user/somepage%3Fvar%3D1 which of course doesn't work.