Merge pull request #1774 from manics/binderhub-client-js-query-url

Do not escape query included in url launch parameter

js/packages/binderhub-client/lib/index.js

@@ -118,30 +118,30 @@ export class BinderRepository {
  // Make a copy of the URL so we don't mangle the original
  let url = new URL(serverUrl);
  if (path) {
- // strip trailing / from URL
- url.pathname = url.pathname.replace(/\/$/, "");
-
+ // Ensure there is a trailing / in serverUrl
+ if (!url.pathname.endsWith("/")) {
+ url.pathname += "/";
+ }
  // trim leading '/' from path to launch users into
  path = path.replace(/(^\/)/g, "");
 
  if (pathType === "lab") {
  // The path is a specific *file* we should open with JupyterLab
-
  // trim trailing / on file paths
  path = path.replace(/(\/$)/g, "");
 
  // /doc/tree is safe because it allows redirect to files
- url.pathname = url.pathname + "/doc/tree/" + encodeURI(path);
+ url = new URL("doc/tree/" + encodeURI(path), url);
  } else if (pathType === "file") {
  // The path is a specific file we should open with *classic notebook*
 
  // trim trailing / on file paths
  path = path.replace(/(\/$)/g, "");
- // /tree is safe because it allows redirect to files
- url.pathname = url.pathname + "/tree/" + encodeURI(path);
+
+ url = new URL("tree/" + encodeURI(path), url);
  } else {
  // pathType is 'url' and we should just pass it on
- url.pathname = url.pathname + "/" + path;
+ url = new URL(path, url);
  }
  }
 

js/packages/binderhub-client/tests/index.test.js

@@ -154,6 +154,26 @@ test("Get full redirect URL and deal with excessive slashes (with pathType=lab)"
  );
 });
 
+test("Get full redirect URL and deal with missing trailing slash", () => {
+ const br = new BinderRepository(
+ "gh/test/test",
+ new URL("https://test-binder.org/build"),
+ );
+ expect(
+ br
+ .getFullRedirectURL(
+ // Missing trailing slash here should not affect target url
+ "https://hub.test-binder.org/user/something",
+ "token",
+ "/directory/index.ipynb/",
+ "lab",
+ )
+ .toString(),
+ ).toBe(
+ "https://hub.test-binder.org/user/something/doc/tree/directory/index.ipynb?token=token",
+ );
+});
+
 test("Get full redirect URL and deal with excessive slashes (with pathType=file)", () => {
  const br = new BinderRepository(
  "gh/test/test",
@@ -233,3 +253,45 @@ describe(
  },
  10 * 1000,
 );
+
+test("Get full redirect URL and deal with query and encoded query (with pathType=url)", () => {
+ const br = new BinderRepository(
+ "gh/test/test",
+ new URL("https://test-binder.org/build"),
+ );
+ expect(
+ br
+ .getFullRedirectURL(
+ "https://hub.test-binder.org/user/something/",
+ "token",
+ // url path here is already url encoded
+ "endpoint?a=1%2F2&b=3%3F%2F",
+ "url",
+ )
+ .toString(),
+ ).toBe(
+ // url path here is exactly as encoded as passed in - not *double* encoded
+ "https://hub.test-binder.org/user/something/endpoint?a=1%2F2&b=3%3F%2F&token=token",
+ );
+});
+
+test("Get full redirect URL with nbgitpuller URL", () => {
+ const br = new BinderRepository(
+ "gh/test/test",
+ new URL("https://test-binder.org/build"),
+ );
+ expect(
+ br
+ .getFullRedirectURL(
+ "https://hub.test-binder.org/user/something/",
+ "token",
+ // urlpath is not actually url encoded - note that / is / not %2F
+ "git-pull?repo=https://github.com/alperyilmaz/jupyterlab-python-intro&urlpath=lab/tree/jupyterlab-python-intro/&branch=master",
+ "url",
+ )
+ .toString(),
+ ).toBe(
+ // generated URL path here *is* url encoded
+ "https://hub.test-binder.org/user/something/git-pull?repo=https%3A%2F%2Fgithub.com%2Falperyilmaz%2Fjupyterlab-python-intro&urlpath=lab%2Ftree%2Fjupyterlab-python-intro%2F&branch=master&token=token",
+ );
+});