Ensure that the path that will ultimately be passed to drupal_encode_…

[alt36]

…path() is not URI-encoded.

With reference to issue #48 . (N.B. I've a feeling this might resolve #46 too)

I think this /may/ have been introduced as a result of 3fa5b82#diff-a60b146b7380729dffb79ce6bbdedbe8

I think the issue is the drupal_goto call at

drupal-raven/raven.module

Line 163 in 75fb441

drupal_goto(get_raven_url(), array('query' => $params), 303);

drupal_goto() in turn just calls url(), and that in turn builds an internal variable called $path and passes it to drupal_encode_path() . Per https://api.drupal.org/api/drupal/includes!common.inc/function/drupal_encode_path/7.x , "a path passed to that function should not be encoded in advance"

[alt36]

..with reference to #46, note that Raven's 'params' query parameter that arises from accessing
http://www.ch.cam.ac.uk/intranet/system/chemistry_raven_downloads/foo%20bar?baz=foo
gets similarly mangled to %253Fbaz%253Dfoo due to re-encoding of % characters

[JeebsUK]

Just looking at both this and #46 with @JKingsnorth's proposed changes, that seems to be suggesting the opposite of what you have proposed in this instance. Do you think that the #46 changes are still necessary on top or are you thinking this will fix both?

[alt36]

I think this PR will fix the issue described in #46 (demo links in a moment...): in both that issue and this one, the underlying issue is that the params query parameter passed to Raven gets (incorrectly) doubly URI-encoded. #47 takes the approach of being more permissive in the response from Raven; this PR should ensure Raven is properly told where to return to thus making the workaround in #47 unnecessary.

Here are two URLs that will auto-redirect you to Raven but then provoke a "Raven authentication failure" (one being a filename with a space, the other having a query parameter a la #46), on a site without this patch applied:

http://www.ch.cam.ac.uk/teaching/files/raven/test%20file.txt
http://www.ch.cam.ac.uk/teaching/files/raven/testfile.txt?foo=bar

and then the equivalent on a site with the patch applied (should see "hello world" in both cases):

http://www.ch.cam.ac.uk/computing/files/raven/test%20file.txt
http://www.ch.cam.ac.uk/computing/files/raven/testfile.txt?foo=bar

(NB in both cases the /files/raven/ part of the path will be rewritten to /system/chemistry_raven_downloads/ ; that's to be expected and is how I enforce Drupal's access checks by forcing you to go through a path handled by hook_menu() )

[JeebsUK]

The top 2 links (to ch.cam.ac.uk/teaching) are also returning hello world for me - is that because it's picked up on my existing raven session / should I be fully logged out for testing purposes?

[alt36]

Yes (see also the more detailed description in #48): the bug here is specifically triggered only when attempting to log in, and thus the Raven-protected URL (e.g. the various demo URLs I posted) gets passed as a parameter to Raven (i.e. when you get taken to https://raven.cam.ac.uk/$parameters_generated_by_thing_making_the_raven_request). If you already have an open Raven session then there's no problem.