Fix getDownloadURL to properly append the xsrf token without a slash.

Before, we would get URLs like http://example.com/files/file.txt/?_xsrf=token, and now the slash after file.txt is eliminated.
jasongrout · Aug 15, 2019 · 9043574 · 9043574
1 parent 860877e
commit 9043574
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 6 deletions.
diff --git a/packages/attachments/src/model.ts b/packages/attachments/src/model.ts
@@ -374,6 +374,9 @@ export class AttachmentsResolver implements IRenderMime.IResolver {
 
   /**
    * Get the download url of a given absolute server path.
+   *
+   * #### Notes
+   * The returned URL may include a query parameter.
    */
   getDownloadUrl(path: string): Promise<string> {
     if (this._parent && !path.startsWith('attachment:')) {

diff --git a/packages/filebrowser/src/model.ts b/packages/filebrowser/src/model.ts
@@ -324,11 +324,8 @@ export class FileBrowserModel implements IDisposable {
     const url = await this.manager.services.contents.getDownloadUrl(path);
     let element = document.createElement('a');
     document.body.appendChild(element);
-    element.setAttribute('href', url);
-    // Chrome doesn't get the right name automatically
-    const parts = path.split('/');
-    const name = parts[parts.length - 1];
-    element.setAttribute('download', name);
+    element.href = url;
+    element.download = '';
     element.click();
     document.body.removeChild(element);
     return void 0;

diff --git a/packages/rendermime-interfaces/src/index.ts b/packages/rendermime-interfaces/src/index.ts
@@ -340,6 +340,9 @@ export namespace IRenderMime {
 
     /**
      * Get the download url for a given absolute url path.
+     *
+     * #### Notes
+     * This URL may include a query parameter.
      */
     getDownloadUrl(url: string): Promise<string>;
 

diff --git a/packages/rendermime/src/registry.ts b/packages/rendermime/src/registry.ts
@@ -328,6 +328,9 @@ export namespace RenderMimeRegistry {
 
     /**
      * Get the download url of a given absolute url path.
+     *
+     * #### Notes
+     * This URL may include a query parameter.
      */
     getDownloadUrl(url: string): Promise<string> {
       if (this.isLocal(url)) {

diff --git a/packages/services/src/contents/index.ts b/packages/services/src/contents/index.ts
@@ -276,6 +276,9 @@ export namespace Contents {
      *
      * @param A promise which resolves with the absolute POSIX
      *   file path on the server.
+     *
+     * #### Notes
+     * The returned URL may include a query parameter.
      */
     getDownloadUrl(path: string): Promise<string>;
 
@@ -420,6 +423,9 @@ export namespace Contents {
      *
      * @param A promise which resolves with the absolute POSIX
      *   file path on the server.
+     *
+     * #### Notes
+     * The returned URL may include a query parameter.
      */
     getDownloadUrl(localPath: string): Promise<string>;
 
@@ -690,6 +696,8 @@ export class ContentsManager implements Contents.IManager {
    *
    * #### Notes
    * It is expected that the path contains no relative paths.
+   *
+   * The returned URL may include a query parameter.
    */
   getDownloadUrl(path: string): Promise<string> {
     let [drive, localPath] = this._driveForPath(path);
@@ -1041,13 +1049,17 @@ export class Drive implements Contents.IDrive {
    *
    * #### Notes
    * It is expected that the path contains no relative paths.
+   *
+   * The returned URL may include a query parameter.
    */
   getDownloadUrl(localPath: string): Promise<string> {
     let baseUrl = this.serverSettings.baseUrl;
     let url = URLExt.join(baseUrl, FILES_URL, URLExt.encodeParts(localPath));
     const xsrfTokenMatch = document.cookie.match('\\b_xsrf=([^;]*)\\b');
     if (xsrfTokenMatch) {
-      url = URLExt.join(url, `?_xsrf=${xsrfTokenMatch[1]}`);
+      const fullurl = new URL(url);
+      fullurl.searchParams.append('_xsrf', xsrfTokenMatch[1]);
+      url = fullurl.toString();
     }
     return Promise.resolve(url);
   }