Link handling differs from Drupal 8's convention

olivierh65 → closed merge request !2

Closing. As noted changes already in 2.x

I believe that actually this is still an issue in the module as such.

The code in the module looks like this at the moment:

$destination = urldecode($result['destination']);
$destination = (strpos($destination, 'http://') !== FALSE || strpos($destination, 'https://') !== FALSE) ?
   $destination :
   Url::fromUri('internal:/' . $destination, ['absolute' => TRUE])->toString();

return $destination;

This means that if the $destination parameter has a leading / (which is the Drupal 8 default) then we're going to try and call fromUri on strings like: internal://something#fragment, which essentially resolves to: /#fragment. This ends up in a redirect loop to the auto login URL, and so your visitors get stuck one way or another.

Since the module is still in alpha, I reckon we should change the API on \Drupal\auto_login_url\AutoLoginUrlCreate::create so that we accept a proper \Drupal\Core\Url object, which we can then do whatever we need to do internally, and save the caller from needing to know what we're going to do.

At the moment, I'm having to do things like this when creating links:

$review_path = Url::fromRoute('entity.commerce_product.canonical', ['commerce_product' => $product->id()], ['fragment' => 'review-form'])
  ->toString();
$login_link = $this->autoLogin->create($order->getCustomerId(), ltrim($review_path, '/'), TRUE);

Which is pretty ugly!

It would be much nicer if I could do:

$review_route = Url::fromRoute('entity.commerce_product.canonical', ['commerce_product' => $product->id()], ['fragment' => 'review-form']);
$login_link = $this->autoLogin->create($order->getCustomerId(), $review_route, TRUE);

Alternatively, if we don't want to change the API, then one of the solutions as outlined on this ticket already, picking up on a leading / and handling that better would work too.