In oauth_commons.pages.inc on line 134, you'll find

  else {
    $query = $_GET;
    unset($query['q']); // why are there so few q's?
    drupal_goto('user/login', array(
      'destination' => url('oauth/authorize', array(
        'query' => $query,
        'absolute' => FALSE,
      )),
    ));
  }

A problem is introduce by the combination of using drupal_goto() and url() with in both cases $query as an array.

In url() this happens with $query as an array:

  if (is_array($options['query'])) {
    $options['query'] = drupal_query_string_encode($options['query']);
  }

The drupal_query_string_encode() is called twice which causes a double encoding.

The fix:

  else {
    $query = $_GET;
    unset($query['q']); // why are there so few q's?
    drupal_goto('user/login', 'destination=' . url('oauth/authorize', 
      array(
        'query' => $query,
        'absolute' => FALSE,
      )
    ));
  }

Find patch attached. (applies by both 6.x-2.x and 6.x-1.x

CommentFileSizeAuthor
oauth_commons.pages-double-encode.patch567 bytestoemaz

Comments

toemaz’s picture

toemaz commented
Status: Active » Needs review

Changing to right status

toemaz’s picture

toemaz commented
Project: OAuth Common (deprecated) » OAuth 1.0
Version: 6.x-2.x-dev » 6.x-3.x-dev

Moving from oauth_common to oauth as advised by voxpelli. Patch still applies.

toemaz’s picture

toemaz commented

Just to make sure that the problem of the double encoding is understood, this is how double encoded destination url looks like:

http://yourdomain.com/user/login?destination=%2Foauth%2Fauthorize%3Foaut...

voxpelli’s picture

voxpelli commented

This seems to somehow be related to #922110: user/login destination parameter incorrectly including base directory on authorize page - right? Should the patches be merged or does one replace the other?

miqmago’s picture

miqmago commented
Version: 6.x-3.x-dev » 7.x-3.x-dev

I'm having same problem on drupal 7. Patch doesn't solve.

Getting the following error after the patch here: Recoverable fatal error: Argument 2 passed to drupal_goto() must be an array, string given, called in C:\Users\Miquel\Documents\00\03-www\public_html\sites\all\modules\oauth\oauth_common.pages.inc on line 236 and defined en drupal_goto() (línea 668 de C:\Users\Miquel\Documents\00\03-www\public_html\includes\common.inc).

#922110 Also not solving the problem.

Candidate solution:

Seems that changing from:

    drupal_goto('user/login', array('query' => array(
      'destination' => url('oauth/authorize', array(
        'query' => $query,
      )),
    )));

to:

    drupal_goto('user/login', array('query' => array(
      'destination' => 'oauth/authorize?' . drupal_http_build_query($query),
      ),
    ));

solves the problem. If it is a good solution I can post the patch.
I'm not sure if 'oauth/authorize?' . drupal_http_build_query() could affect anywherelse.

chustedde’s picture

chustedde commented

I was having the issue in #922110: user/login destination parameter incorrectly including base directory on authorize page (but in D7), and I was able to fix it using your solution, so I posted a patch there.