Merge pull request #1238 from cdrini/fix/urlencode-pagenum

Encode page number in url

Author: cdrini

src/BookReader.js

@@ -1788,7 +1788,7 @@ BookReader.prototype.paramsFromFragment = function(fragment) {
   // Index and page
   if ('undefined' != typeof(urlHash['page'])) {
     // page was set -- may not be int
-    params.page = urlHash['page'];
+    params.page = decodeURIComponent(urlHash['page']);
   }
 
   // $$$ process /region
@@ -1820,11 +1820,10 @@ BookReader.prototype.paramsFromFragment = function(fragment) {
  * @return {string}
  */
 BookReader.prototype.fragmentFromParams = function(params, urlMode = 'hash') {
-  const separator = '/';
   const fragments = [];
 
   if ('undefined' != typeof(params.page)) {
-    fragments.push('page', params.page);
+    fragments.push('page', encodeURIComponent(params.page));
   } else {
     if ('undefined' != typeof(params.index)) {
       // Don't have page numbering but we do have the index
@@ -1850,10 +1849,10 @@ BookReader.prototype.fragmentFromParams = function(params, urlMode = 'hash') {
 
   // search
   if (params.search && urlMode === 'hash') {
-    fragments.push('search', params.search);
+    fragments.push('search', utils.encodeURIComponentPlus(params.search));
   }
 
-  return utils.encodeURIComponentPlus(fragments.join(separator)).replace(/%2F/g, '/');
+  return fragments.join('/');
 };
 
 /**

src/plugins/url/UrlPlugin.js

@@ -45,7 +45,7 @@ export class UrlPlugin {
 
     const strPathParams = this.urlSchema
       .filter(s => s.position == 'path')
-      .map(schema => pathParams[schema.name] ? `${schema.name}/${pathParams[schema.name]}` : '')
+      .map(schema => pathParams[schema.name] ? `${schema.name}/${encodeURIComponent(pathParams[schema.name])}` : '')
       .join('/');
 
     // replace consecutive slashes with a single slash + remove trailing slashes
@@ -83,15 +83,13 @@ export class UrlPlugin {
         const hasPropertyKey = doesKeyExists(urlStrSplitSlashObj, schema.name);
         const hasDeprecatedKey = doesKeyExists(schema, 'deprecated_for') && hasPropertyKey;
 
-        if (hasDeprecatedKey) {
-          urlState[schema.deprecated_for] = urlStrSplitSlashObj[schema.name];
+        // Not in the URL
+        if (!hasPropertyKey && !hasDeprecatedKey) {
           return;
         }
 
-        if (hasPropertyKey) {
-          urlState[schema.name] = urlStrSplitSlashObj[schema.name];
-          return;
-        }
+        const urlStateParam = hasDeprecatedKey ? schema.deprecated_for : schema.name;
+        urlState[urlStateParam] = decodeURIComponent(urlStrSplitSlashObj[schema.name]);
       });
 
     // Add searchParams to urlState

tests/jest/plugins/url/UrlPlugin.test.js

@@ -19,6 +19,10 @@ describe('UrlPlugin tests', () => {
       expect(urlPlugin.urlStateToUrlString(urlStateWithQueries)).toBe(expectedUrlFromStateWithQueries);
     });
 
+    test('encodes page number', () => {
+      expect(urlPlugin.urlStateToUrlString({ page: '12/46' })).toBe(`page/12%2F46`);
+    });
+
     test('urlStateToUrlString with unknown states in schema', () => {
       const urlState = { page: 'n7', mode: '1up' };
       const urlStateWithQueries = { page: 'n7', mode: '1up', q: 'hello', viewer: 'theater', sortBy: 'title_asc' };
@@ -47,6 +51,10 @@ describe('UrlPlugin tests', () => {
       expect(urlPlugin.urlStringToUrlState(url1)).toEqual({page: 'n7', mode: '1up'});
     });
 
+    test('decodes page number', () => {
+      expect(urlPlugin.urlStringToUrlState('/page/12%2F46')).toStrictEqual({ page: '12/46' });
+    });
+
     test('urlStringToUrlState with deprecated_for', () => {
       const url = '/page/n7/mode/2up/search/hello';