Merge pull request #9978 from jasongrout/hash

Fix escaping of urls and paths

packages/rendermime/src/registry.ts

@@ -364,14 +364,14 @@ export namespace RenderMimeRegistry {
      * Get the download url of a given absolute url path.
      *
      * #### Notes
-     * This URL may include a query parameter.
+     * The returned URL may include a query parameter.
      */
-    async getDownloadUrl(url: string): Promise<string> {
-      if (this.isLocal(url)) {
+    async getDownloadUrl(urlPath: string): Promise<string> {
+      if (this.isLocal(urlPath)) {
         // decode url->path before passing to contents api
-        return this._contents.getDownloadUrl(decodeURI(url));
+        return this._contents.getDownloadUrl(decodeURIComponent(urlPath));
       }
-      return url;
+      return urlPath;
     }
 
     /**

packages/rendermime/src/renderers.ts

@@ -803,7 +803,7 @@ namespace Private {
       .resolveUrl(href)
       .then(urlPath => {
         // decode encoded url from url to api path
-        const path = decodeURI(urlPath);
+        const path = decodeURIComponent(urlPath);
         // Handle the click override.
         if (linkHandler) {
           linkHandler.handleLink(anchor, path, hash);

packages/rendermime/test/registry.spec.ts

@@ -443,11 +443,11 @@ describe('rendermime/registry', () => {
         });
 
         it('should resolve URLs with escapes', async () => {
-          expect(await resolverSession.resolveUrl('has%20space')).toContain(
-            urlParent + '/has%20space'
-          );
-          expect(await resolverPath.resolveUrl('has%20space')).toContain(
-            urlParent + '/has%20space'
+          expect(
+            await resolverSession.resolveUrl('has%20space%23hash')
+          ).toContain(urlParent + '/has%20space%23hash');
+          expect(await resolverPath.resolveUrl('has%20space%23hash')).toContain(
+            urlParent + '/has%20space%23hash'
           );
         });
       });
@@ -461,8 +461,10 @@ describe('rendermime/registry', () => {
         });
 
         it('should resolve escapes correctly', async () => {
-          const contextPromise = resolverPath.getDownloadUrl('foo%2520test');
-          const contentsPromise = contents.getDownloadUrl('foo%20test');
+          const contextPromise = resolverPath.getDownloadUrl(
+            'foo%20%23%2520test'
+          );
+          const contentsPromise = contents.getDownloadUrl('foo #%20test');
           const values = await Promise.all([contextPromise, contentsPromise]);
           expect(values[0]).toBe(values[1]);
         });