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

Node IDs with / leads to redirect loop when scope is mounted under a path with slash redirect #1335

Closed
paulbellamy opened this issue Apr 18, 2016 · 3 comments
Assignees
Labels
bug Broken end user or developer functionality; not working as the developers intended it component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer
Milestone

Comments

@paulbellamy
Copy link
Contributor

paulbellamy commented Apr 18, 2016

Components you need are:

  1. A load balancer in front of scope, which mounts it on a subdirectory, and redirects to add a trailing slash to scope's root url.
  2. A node with a / in it's id (or label?) docker run -dit -e MARATHON_APP_ID=a/foo --name foo alpine /bin/sh
  3. open a terminal on that node
  4. close the terminal
  5. observe redirect loop
@paulbellamy paulbellamy added bug Broken end user or developer functionality; not working as the developers intended it component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer labels Apr 18, 2016
@paulbellamy paulbellamy changed the title Node IDs starting with / leads to redirect loop when scope is mounted under a path with slash redirect Node IDs with / leads to redirect loop when scope is mounted under a path with slash redirect Apr 18, 2016
@paulbellamy
Copy link
Contributor Author

Also, just opening details panel and refreshing.

@foot
Copy link
Contributor

foot commented Apr 18, 2016

Is visionmedia/page.js#187

it fails to match either page('/'... or page('/state/:state'..., (because of the slash in the state). and the default fallback strips out the slash for some reason and makes a redirect mess.

but double uri encoding does work!

diff --git a/client/app/scripts/utils/router-utils.js b/client/app/scripts/utils/router-utils.js
index df39d11..8bcddf7 100644
--- a/client/app/scripts/utils/router-utils.js
+++ b/client/app/scripts/utils/router-utils.js
@@ -14,12 +14,12 @@ function shouldReplaceState(prevState, nextState) {

 export function updateRoute() {
   const state = AppStore.getAppState();
-  const stateUrl = JSON.stringify(state);
+  const stateUrl = encodeURIComponent(encodeURIComponent(JSON.stringify(state)));
   const dispatch = false;
   const urlStateString = window.location.hash
     .replace('#!/state/', '')
     .replace('#!/', '') || '{}';
-  const prevState = JSON.parse(decodeURIComponent(urlStateString));
+  const prevState = JSON.parse(decodeURIComponent(decodeURIComponent(urlStateString)));

   if (shouldReplaceState(prevState, state)) {
     // Replace the top of the history rather than pushing on a new item.

@errordeveloper
Copy link
Contributor

Ok, turns out we fixed the issue for nodes with a leading slash, but ones with more slashed (like /monsterz/apps/dnm) still break it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken end user or developer functionality; not working as the developers intended it component/ui Predominantly a front-end issue; most/all of the work can be completed by a f/e developer
Projects
None yet
Development

No branches or pull requests

4 participants