-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
SecurityError when frontend url !== backend url (even with basename) #3241
Comments
Thanks for your question! We want to make sure that the GitHub issue tracker remains the best place to track bug reports and feature requests that affect the development of React Router, and per the issue template, we are not handling support requests here. If you have a question or otherwise need help, please post on Stack Overflow with the #react-router tag at https://stackoverflow.com/questions/ask?tags=react-router, or drop in on the #react-router room on Reactiflux at https://discord.gg/0ZcbPKXt5bYaNQ46. |
@taion I understand the pain of maintaining such a project, and read the things on twitter about the template, and plugin to automatically close issues with message and things like that, but I don't really feel I fall under that case... I took a lot of time to try to make my issue clear. At first I thought it was an issue with npm and the history module not being deduplicated... Made some more tests... Read the source code of react-router and history and did some tweaks in local... I would have liked to provide a JsFiddle to reproduce that bug, but as far as I know it's not really possible to use the Also want to point out that I did found the problem in the source code, and provided a solution which consist of using a fully qualified absolute url in the path of I don't see where you think I'm asking for support, I am here to help actually. If I did not make a PR it's only because I don't know the internals enough yet and would like guidance on how to solve my problem in a proper way (because So yes, I feel a bit frustrated that you close this issue so fast without any additional comment :( I feel like future contributors are not really welcome... (because yes, I want to do a PR to solve that problem!) |
There's no bot or plugin here. I read through your issue, as I read through all the issues. You're using some sort of extremely odd setup here that touches on a number of things that are not explicitly supported. As I said above, please reach out to one of the support forums for help with your setup. This does not look like a bug in React Router or in history. |
@taion thanks for your comment.
Are you refering to I would argue that for me using
Code directly taken from // Automatically use the value of <base href> in HTML
// documents as basename if it's not explicitly given.
if (basename == null && canUseDOM) {
const base = document.getElementsByTagName('base')[0]
if (base)
basename = extractPath(base.href)
} According to the source code I see here, it is clearly expected to support a
@taion clearly I can't post anything to any of these places, because what is the point of asking a question for my setup if React-router does not intend to support that setup? People can only tell me to use something else that will support my usecase... So the question is: do react-router want to support If you don't want to support this usecase (or maybe not in this version) just tell me and I'll hack a fork for myself and that's all, but for me I'm really not doing anything fancy. If you want to support it then maybe can you help me make a PR. |
Again, as noted in the project description, for questions and support, please visit our channel on Reactiflux or Stack Overflow. The issue tracker is exclusively for bug reports and feature requests. |
Seems like |
Not sure if I fully understand what's going on but I think this will work for you?
|
Hi, @ryanflorence thanks for reopening this. I've made a reproductible sandbox based on the "active-links" example. The basename attribute works, when you click on a link there is no error. The problem is that on app mounting, React-router tries to replace current history state but does not use the basename in that step. I also made a pull-request with what seems needed to solve my problem but I'm not sure of the impacts it may have. Note I've done it against History 2.x as it seems 3.x is quite different. Usecase for using different domains:
|
…replaceState to let it unchanged See remix-run/react-router#3241
React Router currently doesn't allow linking to other domains. Also, For now, set You can follow along at remix-run/history#94. |
@taion you maybe misunderstood the issue because I am NOT trying to link to another domain. My backend is I use ReactRouter relative Links like The only problem is that my index.html page served by the backend have to define a Because of this base href, the calls history makes to So I'm using the following let history = useRouterHistory(createHistory)({
basename: (window.location.protocol + "//" + window.location.hostname + ":" + window.location.port + "/")
}); With this setup, it actually makes all the The only problem is on History startup because of this line: @taion I wiill stop the discussion on this issue, as finally it's more a History.js problem and no code in React-Router should be changed, and will continue discussion on remix-run/history#267 and remix-run/history#94 |
Let's continue to discuss this here, since there's more context available here. This might be a history problem, but it's not one we can fix. Your current setup is that you have:
You have this because you want something like However, history is also interpreting This is a sort of inconvenient bug in history. We can't fix this without a breaking release. Instead, if you set |
@taion the problem is not about the basename being inferred for me. The problem is that on history creation / init, the history tries to do a
Really, just changing that line when you first put the key on current history entry solves my problem :) If one does not need to change the current url, but only the state, one could simply omit the last parameter and should produce the same result. I think the point of this code is to lazily put the history key inside history state, so removing the last parameter looks safe to me. A note on using base-href=otherDomainIt is important to note that once a base href has been set to some other domain, then any call to let history = useRouterHistory(createHistory)({
basename: (window.location.protocol + "//" + window.location.hostname + ":" + window.location.port + "/")
}); This code solves my problem, because when doing |
History does not use the value of If you pass in |
@taion basename = '' does not work. Let's not focus on automatic basename inference because I override it anyway (weither it's by the solution I showed you, or with an empty string) My link has always been relative: Let's consider I never change this link, and see how my basename compare to your basename. Case 1:
|
When initializing history with initial state, do not provide path to replaceState to let it unchanged See remix-run/react-router#3241
I see. Thanks for explaining, and for setting up a fix. I merged your PR on history. |
@taion thanks! :) By chance do you plan a 2.0.2 release soon? |
We'll try. Thanks! |
ok thanks ping me if you release :) |
I just cut a history 2.0.2 release. Thanks for the fix, @slorber 💃 |
awesome thanks :) |
For the record, it's been reported by my team that when a base href of different origin is set, the following fails for Safari: It can probably be fixed by always providing an absolute url as path. Imho it works on all recent browsers but It's worth testing on older versions I guess. Can you run such tests? My usecase is for remote dev only as I explained so I can live with it only working in Chrome but if other users are interested in the same kind of stuff working on Safari I'm just reporting the issue |
Note that regression test of @taion seems to have catched some issues with Safari: remix-run/history#274 Note sure it'll fix the problem but it's worth trying to run CI against that: remix-run/history@v2.x...slorber:patch-1 |
@slorber FWIW the 3.x branch shouldn't have this issue at all because we don't issue a |
great :) is History 3.x compatible with currently released react-router or the 2 should be upgraded at the same time? |
@mjackson Don't we still need to get the initial location, and to stamp the location key onto the history state storage when initially rendering the router? |
@slorber Isn't that just |
Version
2.0.0
Steps to reproduce
<base href="http://localhost:8080"/>
Expected Behavior
React-router should work, or at least it should be possible to configure it to make it work
Actual Behavior
Trying with basename:
Same error.
Solution proposal
It seems the error is here:
_DOMUtils.getWindowPath();
returns a relative url.In my experience encoutering a similar problem in the past, when base attribute is set, we should always use absolute urls. You can see it by yourself by running a simple test on a page which have a base attribute:
Here is the original implementation that returns a relative url
By changing it to:
it solves my problem and I don't have security issues anymore. However I have no idea of what else in React-router it could break.
Now I have this warning but it seems to start at least:
Warning: A path must be pathname + search + hash only, not a fully qualified URL like "http://localhost:9000/public/stamples/56fd5227be7e3a4f0770c486/slug"
Not really sure it's a History or React-router, feel free to move it needed ;)
The text was updated successfully, but these errors were encountered: