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

replaceState Error when path starts with // and scrollBehavior is set #2593

Closed
yegorLitvinov opened this issue Jan 22, 2019 · 10 comments · Fixed by #3652 · May be fixed by #3679
Closed

replaceState Error when path starts with // and scrollBehavior is set #2593

yegorLitvinov opened this issue Jan 22, 2019 · 10 comments · Fixed by #3652 · May be fixed by #3679
Labels
bug fixed on 4.x This issue has been already fixed on the v4 but exists in v3

Comments

@yegorLitvinov
Copy link

Version

3.0.2

Reproduction link

https://github.com/yegorLitvinov/vue-router-bug

Steps to reproduce

Set scrollBehavior property in router config. Put two forward slashes into beginning of the url (e.g. http://localhost:8081//about)

What is expected?

No errors in console

What is actually happening?

In firefox: SecurityError: The operation is insecure.
In Chrome: Uncaught DOMException: Failed to execute 'replaceState' on 'History': A history state object with URL 'http://about/' cannot be created in a document with origin 'http://localhost:8081' and URL 'http://localhost:8081//about'

@posva posva changed the title SecurityError when url contains // and scrollBehavior is set replaceState Error when url contains // and scrollBehavior is set Jan 22, 2019
@posva posva added the bug label Jan 22, 2019
@posva posva changed the title replaceState Error when url contains // and scrollBehavior is set replaceState Error when path starts with // and scrollBehavior is set Jan 22, 2019
@posva posva removed the bug label Jan 22, 2019
@posva
Copy link
Member

posva commented Jan 22, 2019

It looks like we should sanitize the beginning of the URL so it doesn't contain trailing slashes but I'm still unsure about it. I will have to search a bit more

@luatnd
Copy link

luatnd commented May 21, 2019

I faced this issue too.

Steps to reproduce:

  1. Build
  2. deploy to primary.domain.com
  3. deploy to secondary.domain.com
  4. Add <base href="https://primary.domain.com" target="_blank"> into secondary.domain.com
  5. Access url secondary.domain.com then saw this issue

Debug info

vue: 2.5.17
vue-router: 3.0.6,

vue-router/dist/vue-router.js:1550

function setupScroll () {
  // Fix for #1585 for Firefox
  // Fix for #2195 Add optional third attribute to workaround a bug in safari https://bugs.webkit.org/show_bug.cgi?id=182678
  window.history.replaceState({ key: getStateKey() }, '', window.location.href.replace(window.location.origin, '')); // ======> This line cause the issue
  window.addEventListener('popstate', function (e) {
    saveScrollPosition();
    if (e.state && e.state.key) {
      setStateKey(e.state.key);
    }
  });
}

The temporary solution:
Comment the bellow line to avoid setupScroll => Then implement scrollBehavior by your self on route changed:

const Router = new VueRouter({
    scrollBehavior: () => ({ y: 0 }), // ===> Comment this line to avoid setupScroll => Then implement scrollbehavior by your self on route change.
    routes: ...,
  })

If you ignore scrollBehavior key, setupScroll() will not be executed.

Hope this help!

@bkarlson
Copy link

bkarlson commented Nov 7, 2019

wow, thanks for this issue. Just faced this as well, exact same scenario as @luatnd desribed.

@ralf57
Copy link

ralf57 commented Nov 20, 2019

@posva any news about this?

@Aigrefin
Copy link

Aigrefin commented Feb 3, 2020

Hi,
The temporary solution from @luatnd doesn't always work.
Any progress on this bug ?

@Madd0g
Copy link

Madd0g commented Feb 21, 2020

I'm also seeing this problem, but with $ sign in the querystring.

@mattheyan
Copy link

I'm experiencing this problem as well.

In my case the original URL looks like https://site.com//?x=y, which results in calling replaceState with //?x=y as the URL. I put in a place a workaround to detect the situation and trim the duplicate / before initializing the router.

This comment has some useful info: remix-run/react-router#3184 (comment)
Note that although I'm linking to an issue in a react package, I am actually experiencing this issue in vue router. 😛 It seems this same issue is referenced in lots of different contexts, so it must be a common / easy problem to run into. Either that or different libs have based their browser history code on the same original source.

@posva posva added the fixed on 4.x This issue has been already fixed on the v4 but exists in v3 label May 12, 2020
@HaydenBruin
Copy link

I was having this issue as well. The only thing I was using scrollBehavior for was to scroll the user to the top of the page.

Before:

const router = new VueRouter({
  base: process.env.BASE_URL,
  routes,
  scrollBehavior() {
    return { x: 0, y: 0 };
  },
});

After:

const router = new VueRouter({
  base: process.env.BASE_URL,
  routes,
});

router.afterEach(() => {
  window.scrollTo(0, 0);
});

@posva
Copy link
Member

posva commented Apr 14, 2021

In v4 this is fixed by using the full URL: https://github.com/vuejs/vue-router-next/blob/master/src/history/html5.ts#L220

I think it's worth adding the same fix here

@snoozbuster
Copy link

hey @posva - I don't think this is closed by the linked PR. That PR does fix the issue if the navigation comes from a router-link or programmatic call to replace() or push() - but the issue still remains if the page is hard-loaded with a double-slash in the pathname.

I think the issue is caused by this line, as @mattheyan mentioned. When the pathname contains multiple leading slashes, the absoluteUrl with the scheme and hostname excised contains a leading double slash, which is reinterpreted as an actual absolute URL (with current scheme, host and path) instead of a path on the current domain. cleanPath isn't ever called on this URL, so the fix in that PR doesn't actually fix this issue.

I imagine the easiest way to fix this is to update the code to call cleanPath here. assuming that's the correct solution, I've taken the liberty of opening a PR for it: #3679 (with included e2e test)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fixed on 4.x This issue has been already fixed on the v4 but exists in v3
Projects
None yet
10 participants