Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP 303 Redirects with anchor not respected #211

Open
rmm5t opened this issue Mar 15, 2021 · 17 comments
Open

HTTP 303 Redirects with anchor not respected #211

rmm5t opened this issue Mar 15, 2021 · 17 comments

Comments

@rmm5t
Copy link

rmm5t commented Mar 15, 2021

Steps to Reproduce:

  • Redirect after a form submission (via HTTP 303) to a location that contains an anchor (e.g. Location: /path#anchor).

Observed Behavior:

  • The redirection occurs, but strips the anchor from the resulting page (e.g. /path instead of /path#anchor)
  • This results in any page rendering dependent upon this anchor to fail.

Expected Behavior

  • Redirect and expose the full path with the anchor in the resulting page's address bar.

Workaround

  • Disable turbo on that particular form submission.

Wild Guess Hypothesis
I suspect that the subsequent redirection fetch (from Turbo) strips the anchor tag from the underlying HTTP request (as expected), but when the page is re-hydrated, the address bar URL (pushstate) is not updated to reflect the original anchor.

@geoffreymcgill
Copy link

We have run into what seems to be this exact same issue, but with 301 redirects where the requested URL is missing a trailing /.

We could reproduce by configuring a link + anchor without a trailing /:

<a href="../path#anchor">FAIL</a>

Server responds with a 301 Moved Permanently and redirects to the URL with the trailing / appended, although the #anchor is removed from the URL.

The final expected and actual URL in the browser would be:

// Expected
https://example.com/path/#anchor

// Actual
https://example.com/path/

If the tailing / is manually added to the original link, the anchors are respected and not trimmed from the resulting URL:

<a href="../path/#anchor">PASS</a>

We understand that including a trailing / is best practice, so we're updating our processes to include the trailing /, but still.

@stebo
Copy link

stebo commented May 20, 2021

I can confirm that this is an issue and I find it quite an unexpected behavior and would love to use my anchors.
Adding a tailing /does not solve the issue for me when doing redirects in the controller:

redirect_to '.../posts/433545#comment_221760128', status: :see_other
# gets redirected to /posts/433545

redirect_to '.../posts/433545/#comment_221760128', status: :see_other
# gets redirected to /posts/433545/

In both cases the anchor is lost. Using 301, 302, 303 makes no difference.
The only solution right now is to set data-turbo=falseon the form.

Any ideas how to fix this?

@johnloringpollard
Copy link

I believe this issue occurs for parameters as well. When I redirect with params in the same controller, the params are lost. If I redirect to a different controller, then the params are kept. Very strange!

@nickjj
Copy link

nickjj commented Jun 21, 2021

I'm seeing this behavior when performing any type of redirect after a successful form submission and the problem goes away if I disable Turbo. I'm also not using Rails so this is isolated to the JS library itself. I'm using beta 7 and I'm also not using frames or streams here. This is a basic full blown HTTP redirect.

Reproduceable steps:

  1. import * as Turbo from "@hotwired/turbo";

  2. Create a form where after the POST comes in, redirect to /#foo

  3. The #foo will get stripped out from the redirect

  4. Comment out the line from step 1 and do the same thing and now the anchor is kept in tact.

It's worth pointing out params are kept in tact in either case. I'm only losing the anchor with Turbo.

@tobyzerner
Copy link

tobyzerner commented Nov 11, 2021

I'm not sure how this can be fixed. As discussed in #257 (comment):

A fetch Response resulting in a redirect deliberately prevents access to the intermediate redirect response with a status in the 300...399 range.

This means that the browser itself strips the anchor from the redirection URL, before it opaquely fetches the anchorless URL and hands the result to Turbo. There isn't a way for Turbo to excise the Location header and its anchor from the redirect response.

I think this is the relevant fetch issue: whatwg/fetch#1167

@rubys
Copy link

rubys commented May 23, 2022

While it may not be possible to fix this, at a minimum it would be helpful if Rails were to emit a log message whenever a redirect with an anchor response is produced in response to a request that is being processed as TURBO_STREAM indicating that the anchor will effectively be ignored, and perhaps suggesting data: {turbo: false} as a means to honor this parameter.

Unfortunately, looking at the current code, architecturally this is awkward. actionpack/lib/action_dispatch/http/url.rb is what processes the anchor option and it is entirely unaware of the request.

actionpack/lib/action_controller/metal/redirecting.rb could look at the location header that is produced and is aware of the request, but Rails to date literally has no code that is turbo specific.

Ideally, this would be a part of the hotwired turbo-rails gem, but as near as I can tell, absolutely none of server side logic which produces the redirect response goes through any code that is included in this gem. Perhaps turbo-rails could add some middleware to the processing, but that seems like a heavyweight solution that would penalize all applications in order to handle this one edge case.

If my humble ramblings above spark any sort of ideas on how this situation can be improved, I'll be glad to develop or contribute a pull request to address this.

@dhh
Copy link
Member

dhh commented Jun 18, 2022

Could also just start by adding the caveat to the docs. Explain why it isn't possible to fix.

@bradgessler
Copy link

bradgessler commented Oct 26, 2022

I'm also running into this issue for a Stripe redirection link, as described in the duplicate ticket at #565. I've asked Stripe to consider changing their anchor to an URL param, but I have no idea if that will actually happen in any meaningful amount of time, if at all.

I'm not sure how "edge" of a case this is given the pervasiveness of Stripe, and the criticality of payments for most production web applications.

@bradgessler
Copy link

Ok, I did a lot more work investigating and I think I narrowed it down to a CORS policy. If the URL with an anchor tag emits the header Access-Control-Allow-Origin: *.

For example, this won't redirect to the anchor:

redirect_to "https://checkout.stripe.com/c/pay/cs_test_a1N9yeht6Z860VIwhoLHpJYVRo3Hlr9tIhd1foVLhTofesLGenoGVRcZdJ#anchor", allow_other_host: true

Because it returns a 200 OK with a Access-Control-Allow-Origin: * header

But this will work:

redirect_to "https://checkout.stripe.com/#anchor", allow_other_host: true

Because the URL does not emit a CORS header and the connection gets canceled.

@doits
Copy link

doits commented Dec 13, 2022

Not sure if this is related but I have the same problem with a simple form with an anchor in the action and method="get":

<form action="/some/page#anchor" method="get">
  ...
</form>

Submitting the form (with Turbo enabled) actually sends a request to /some/page (anchor is stripped right away). Without Turbo it works fine.

The server simply responds with status 200 and new html for the page; it updates correctly but does not scroll to the (missing) anchor with Turbo. So there's no redirect happening.

Is this the same problem or something different?

@caseyli
Copy link

caseyli commented Mar 18, 2023

For those trying to execute the Stripe redirect, as mentioned by @rubys, using data: { turbo: false } is a solution that works. Considering Stripe checkout is redirecting to an outside site anyway, not using turbo in this one case is probably not a huge deal. So for those looking to make their Stripe redirect work, try this:

(In this example, we're using a PaymentMethodsController)

<%# new.html.erb, the key here being data: { turbo: false } %>
<%= link_to "Add credit card", new_payment_method_path, class: 'btn btn-primary', data: { turbo: false } %>
class PaymentMethodsController < ApplicationController
  def new
    session_options = {...}
    session = Stripe::Checkout::Session.create(session_options)
    redirect_to session.url, allow_other_host: true
  end
end

Works well for me.

luka-n added a commit to luka-n/sedemnajst that referenced this issue Aug 11, 2023
@krsyoung
Copy link

krsyoung commented Oct 20, 2023

I believe we are hitting this in a rails 7.0.8 app and to confirm that it wasn't something weird that we're doing, I created a repo to reproduce the issue: https://github.com/harled/turbo-anchor-issue

This is a fresh rails 7.1.1 application with a single controller serving an index and redirect endpoint. The summary for what works (and what doesn't):

image

Although the anchor not showing is an issue (users can't copy & paste the page at the right spot), our main problem is actually that Turbo doesn't scroll to the element. GET, despite not updating the URL does indeed scroll correctly.

Thoughts if this PATCH / no scroll issue all part of the same problem or a different issue?

For background, we're using a link to with a turbo PATCH to process end user notifications (they click, notification is marked as read and then they are redirected to the relevant comment - although now they are redirected to the top of the page).

UPDATED: I updated the repo with more test options. One really weird finding an anchor scenario on the link dominating that of the response from the server.

UPDATED: I created a blog post to explain the issue further. In writing it, it seems really odd to me that this is an issue that isn't receiving attention. Are anchors not cool any more!?

UPDATED: It isn't pretty, but I created another blog post with our first stab at a workaround. Not gonna lie, it includes a timer :( However, it does look to work with get / post requests with redirects using anchors and scrolls to the correct location. This is can all be see via https://github.com/harled/turbo-anchor-issue/tree/turbo-anchors

r3trofitted added a commit to r3trofitted/shangrila that referenced this issue Jan 19, 2024
Note an unexpected issue: URL anchors aren't handled by Turbo redirects
(see hotwired/turbo#211), so Turbo has to be
skipped. which is too bad. TODO: find a solution to scroll to the anchor.

(Plus find a way for the edit action to figure out _which anchor_ to
use.)
@radiz13
Copy link

radiz13 commented Feb 21, 2024

Hi ! Im'm using Turbo UX with Symfony 7, I can confirm anchors don't work.
If turbo-core is enable :

$url = $this->generateUrl('back_client_actu_gerer').'#article'.$article->getId(); 
return $this->redirect($url);

redirects me to "/gerer" (with or without "/" before "#")
If turbo-core is disable, no problem, the same command redirects me to "/gerer#article102".
There's no issue ? Thx

@bradgessler
Copy link

Ran into this again. Given this code:

=link_to "Upgrade now →", account_payment_path(@account), "data-turbo-method": "post", class: "btn btn-primary is-medium"

Here's the work around I went with this time:

= form_tag account_payment_path(@account), method: :post, "data-turbo": false do
  = button_tag "Upgrade now →", class: "btn btn-primary btn-lg w-full"

@marckohlbrugge
Copy link

Ran into the same issue, also with Stripe Checkout. This seems like a pretty substantial bug.

Is there a workaround that lets us specify something in the controller action, rather than having to remember to disable turbo for each link that points to this action?

@MolloKhan
Copy link

I discovered today that some of my links have this issue. It seems like a hard thing to fix properly, but does anyone know a workaround? Thanks in advance

@jvillarejo
Copy link

jvillarejo commented Apr 16, 2024

I was implementing a notification sidebar and I got into this problem.

In my implementation some notifications have an anchor in the redirect url so once the browser loads it can scroll to the correct element.

Here is my workaround for anyone that might find it useful.

I had to change the status code from the redirect_to so it doesn't return a 30x code and it returns a 200 instead, so I can workaround the fetch atomic http redirect handling

class RecipientNotificationsController < ApplicationController
  def show
    @recipient_notification.read!
    # Workaround this sends status :ok because of opaque redirect in fetch API
    redirect_to notification_origin_path(anchor: ...), status: :ok
  end
end

Then on the notification card with the link I added a stimulus controller that handles the anchor link

<%= turbo_frame_tag dom_id(recipient_notification) do %>
  <%= link_to recipient_notification_path(recipient_notification), data: {
              turbo_frame: '_top',
              controller: 'redirect-preserving-url-anchor',
              action: 'redirect-preserving-url-anchor#redirect',
              'turbo-prefetch': false
            }, class: 'notification-link' do %>
            
            <% # ... %>
   <% end %>  
<% end %>

And here is the Stimulus contorller redirect_preserving_url_anchor_controller.js

import { Turbo } from '@hotwired/turbo-rails';
import { get } from '@rails/request.js';
import ApplicationController from './application_controller';

export default class extends ApplicationController {
  async redirect(event) {
    event.preventDefault();

    const response = await get(this.element.href);

    if (response.ok) {
      const location = response.headers.get('Location');
      Turbo.visit(location, { action: 'replace' });
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests