Merge pull request #854 from weseek/fix/850-remove-trailing-slack-when-moving

Fix/850 remove trailing slash when moving

CHANGES.md

@@ -10,6 +10,7 @@ CHANGES
 * Fix: Posting comment doesn't work under Crowi Classic Layout
     * Introduced by 3.1.5
 * Fix: HackMD doesn't work when `siteUrl` ends with slash
+* Fix: Ensure not to be able to move/duplicate page to the path which has trailing slash
 * Support: Launch with Node.js v10
 * Support: Launch with MongoDB 3.6
 * Support: Launch with Elasticsearch 6.6

src/client/js/app.js

@@ -281,7 +281,7 @@ const componentMappings = {
   'bookmark-button': <BookmarkButton pageId={pageId} crowi={crowi} />,
   'bookmark-button-lg': <BookmarkButton pageId={pageId} crowi={crowi} size="lg" />,
 
-  'create-page-name-input': <PagePathAutoComplete crowi={crowi} initializedPath={pagePath} addSlashToTheEnd={true} />,
+  'create-page-name-input': <PagePathAutoComplete crowi={crowi} initializedPath={pagePath} addTrailingSlash={true} />,
   'rename-page-name-input': <PagePathAutoComplete crowi={crowi} initializedPath={pagePath} />,
   'duplicate-page-name-input': <PagePathAutoComplete crowi={crowi} initializedPath={pagePath} />,
 

src/client/js/components/PagePathAutoComplete.jsx

@@ -34,9 +34,9 @@ export default class PagePathAutoComplete extends React.Component {
   }
 
   getKeywordOnInit(path) {
-    return this.props.addSlashToTheEnd
-      ? pathUtils.addSlashToTheEnd(path)
-      : pathUtils.removeLastSlash(path);
+    return this.props.addTrailingSlash
+      ? pathUtils.addTrailingSlash(path)
+      : pathUtils.removeTrailingSlash(path);
   }
 
   render() {
@@ -59,7 +59,7 @@ export default class PagePathAutoComplete extends React.Component {
 PagePathAutoComplete.propTypes = {
   crowi:            PropTypes.object.isRequired,
   initializedPath:  PropTypes.string,
-  addSlashToTheEnd: PropTypes.bool,
+  addTrailingSlash: PropTypes.bool,
 };
 
 PagePathAutoComplete.defaultProps = {

src/client/js/legacy/crowi.js

@@ -7,12 +7,12 @@ import ReactDOM from 'react-dom';
 
 import { debounce } from 'throttle-debounce';
 
-const pathUtils = require('@commons/util/path-utils');
 const entities = require('entities');
 const escapeStringRegexp = require('escape-string-regexp');
 require('jquery.cookie');
 require('bootstrap-select');
 
+import * as pathUtils from '@commons/util/path-utils';
 import GrowiRenderer from '../util/GrowiRenderer';
 import RevisionLoader from '../components/Page/RevisionLoader';
 
@@ -356,7 +356,7 @@ $(function() {
     // create name-value map
     let nameValueMap = {};
     $(this).serializeArray().forEach((obj) => {
-      nameValueMap[obj.name] = obj.value; // nameValueMap['q'] is renamed page path
+      nameValueMap[obj.name] = obj.value; // nameValueMap.new_path is renamed page path
     });
 
     const data = $(this).serialize() + `&socketClientId=${crowi.getSocketClientId()}`;
@@ -370,10 +370,11 @@ $(function() {
     .done(function(res) {
       // error
       if (!res.ok) {
+        const linkPath = pathUtils.normalizePath(nameValueMap.new_path);
         $('#renamePage .msg, #unportalize .msg').hide();
         $(`#renamePage .msg-${res.code}, #unportalize .msg-${res.code}`).show();
         $('#renamePage #linkToNewPage, #unportalize #linkToNewPage').html(`
-          <a href="${nameValueMap.new_path}">${nameValueMap.new_path} <i class="icon-login"></i></a>
+          <a href="${linkPath}">${linkPath} <i class="icon-login"></i></a>
         `);
       }
       else {
@@ -394,7 +395,7 @@ $(function() {
     // create name-value map
     let nameValueMap = {};
     $(this).serializeArray().forEach((obj) => {
-      nameValueMap[obj.name] = obj.value; // nameValueMap['q'] is duplicated page path
+      nameValueMap[obj.name] = obj.value; // nameValueMap.new_path is duplicated page path
     });
 
     $.ajax({
@@ -405,10 +406,11 @@ $(function() {
     }).done(function(res) {
       // error
       if (!res.ok) {
+        const linkPath = pathUtils.normalizePath(nameValueMap.new_path);
         $('#duplicatePage .msg').hide();
         $(`#duplicatePage .msg-${res.code}`).show();
         $('#duplicatePage #linkToNewPage').html(`
-          <a href="${nameValueMap.q}">${nameValueMap.q} <i class="icon-login"></i></a>
+          <a href="${linkPath}">${linkPath} <i class="icon-login"></i></a>
         `);
       }
       else {

src/lib/util/path-utils.js

@@ -18,40 +18,63 @@ function encodePagePath(path) {
   return paths.join('/');
 }
 
-function matchEndWithSlash(path) {
-  // https://regex101.com/r/Z21fEd/1
-  return path.match(/(.+?)(\/)?$/);
+function matchSlashes(path) {
+  // https://regex101.com/r/Z21fEd/3
+  return path.match(/^((\/)?(.+?))(\/)?$$/);
 }
 
-function isEndWithSlash(path) {
-  const match = matchEndWithSlash(path);
+function hasHeadingSlash(path) {
+  const match = matchSlashes(path);
   return (match[2] != null);
 }
 
-function addSlashToTheEnd(path) {
+function hasTrailingSlash(path) {
+  const match = matchSlashes(path);
+  return (match[4] != null);
+}
+
+function addHeadingSlash(path) {
+  if (path === '/') {
+    return path;
+  }
+
+  if (!hasHeadingSlash(path)) {
+    return `/${path}`;
+  }
+  return path;
+}
+
+function addTrailingSlash(path) {
   if (path === '/') {
     return path;
   }
 
-  if (!isEndWithSlash(path)) {
+  if (!hasTrailingSlash(path)) {
     return `${path}/`;
   }
   return path;
 }
 
-function removeLastSlash(path) {
+function removeTrailingSlash(path) {
   if (path === '/') {
     return path;
   }
 
-  const match = matchEndWithSlash(path);
+  const match = matchSlashes(path);
   return match[1];
 }
 
+function normalizePath(path) {
+  return this.addHeadingSlash(this.removeTrailingSlash(path));
+}
+
 module.exports = {
   encodePagePath,
   encodePagesPath,
-  isEndWithSlash,
-  addSlashToTheEnd,
-  removeLastSlash,
+  hasHeadingSlash,
+  hasTrailingSlash,
+  addHeadingSlash,
+  addTrailingSlash,
+  removeTrailingSlash,
+  normalizePath,
 };

src/server/models/page.js

@@ -489,16 +489,6 @@ module.exports = function(crowi) {
     return grantLabels;
   };
 
-  pageSchema.statics.normalizePath = function(path) {
-    if (!path.match(/^\//)) {
-      path = '/' + path;
-    }
-
-    path = path.replace(/\/\s+?/g, '/').replace(/\s+\//g, '/');
-
-    return path;
-  };
-
   pageSchema.statics.getUserPagePath = function(user) {
     return '/user/' + user.username;
   };

src/server/routes/page.js

@@ -960,7 +960,7 @@ module.exports = function(crowi, app) {
   api.rename = async function(req, res) {
     const pageId = req.body.page_id;
     const previousRevision = req.body.revision_id || null;
-    const newPagePath = Page.normalizePath(req.body.new_path);
+    const newPagePath = pathUtils.normalizePath(req.body.new_path);
     const options = {
       createRedirectPage: req.body.create_redirect || 0,
       moveUnderTrees: req.body.move_trees || 0,
@@ -1024,7 +1024,7 @@ module.exports = function(crowi, app) {
    */
   api.duplicate = async function(req, res) {
     const pageId = req.body.page_id;
-    const newPagePath = Page.normalizePath(req.body.new_path);
+    const newPagePath = pathUtils.normalizePath(req.body.new_path);
 
     const page = await Page.findByIdAndViewer(pageId, req.user);
 

src/server/service/config-manager.js

@@ -81,7 +81,7 @@ class ConfigManager {
   getSiteUrl() {
     const siteUrl = this.getConfig('crowi', 'app:siteUrl');
     if (siteUrl != null) {
-      return pathUtils.removeLastSlash(siteUrl);
+      return pathUtils.removeTrailingSlash(siteUrl);
     }
     else {
       return '[The site URL is not set. Please set it!]';

src/server/util/middlewares.js

@@ -1,8 +1,10 @@
 const debug = require('debug')('growi:lib:middlewares');
 const logger = require('@alias/logger')('growi:lib:middlewares');
+const pathUtils = require('@commons/util/path-utils');
 const md5 = require('md5');
 const entities = require('entities');
 
+
 exports.csrfKeyGenerator = function(crowi, app) {
   return function(req, res, next) {
     var csrfKey = (req.session && req.session.id) || 'anon';
@@ -145,12 +147,8 @@ exports.swigFilters = function(crowi, app, swig) {
         .replace(/\n/g, '<br>');
     });
 
-    swig.setFilter('removeLastSlash', function(string) {
-      if (string == '/') {
-        return string;
-      }
-
-      return string.substr(0, string.length - 1);
+    swig.setFilter('removeTrailingSlash', function(string) {
+      return pathUtils.removeTrailingSlash(string);
     });
 
     swig.setFilter('presentation', function(string) {

src/server/views/modal/what_is_portal.html

@@ -67,12 +67,12 @@
 
         <strong>Warning!</strong><br>
 
-        <p>既に <strong><a href="{{ path|removeLastSlash }}">{{ path|removeLastSlash }}</a></strong> のページが存在します。</p>
+        <p>既に <strong><a href="{{ path|removeTrailingSlash }}">{{ path|removeTrailingSlash }}</a></strong> のページが存在します。</p>
 
         <p>
-          <a href="{{ path|removeLastSlash }}">{{ path|removeLastSlash }}</a> をポータル化するには、
-          <a href="{{ path|removeLastSlash }}">{{ path|removeLastSlash }}</a> に移動し、「ページを移動」させてください。<br>
-          <a href="{{ path|removeLastSlash }}">{{ path|removeLastSlash }}</a> とは別に、このページ(<code>{{ path }}</code>)にポータルを作成する場合、このまま編集を続けて作成してください。
+          <a href="{{ path|removeTrailingSlash }}">{{ path|removeTrailingSlash }}</a> をポータル化するには、
+          <a href="{{ path|removeTrailingSlash }}">{{ path|removeTrailingSlash }}</a> に移動し、「ページを移動」させてください。<br>
+          <a href="{{ path|removeTrailingSlash }}">{{ path|removeTrailingSlash }}</a> とは別に、このページ(<code>{{ path }}</code>)にポータルを作成する場合、このまま編集を続けて作成してください。
         </p>
 
       </div>

src/test/models/page.test.js

@@ -267,20 +267,6 @@ describe('Page', () => {
     });
   });
 
-  describe('Normalize path', () => {
-    context('Normalize', () => {
-      it('should start with slash', done => {
-        expect(Page.normalizePath('hoge/fuga')).to.equal('/hoge/fuga');
-        done();
-      });
-
-      it('should trim spaces of slash', done => {
-        expect(Page.normalizePath('/ hoge / fuga')).to.equal('/hoge/fuga');
-        done();
-      });
-    });
-  });
-
   describe('.findPage', () => {
     context('findByIdAndViewer', () => {
       it('should find page (public)', async() => {

src/test/util/path-utils.test.js

@@ -0,0 +1,22 @@
+const chai = require('chai')
+  , expect = chai.expect
+  , sinonChai = require('sinon-chai')
+  ;
+chai.use(sinonChai);
+
+const pathUtils = require('@commons/util/path-utils');
+
+describe('page-utils', () => {
+
+  describe('.normalizePath', () => {
+    it('should add heading slash', done => {
+      expect(pathUtils.normalizePath('hoge/fuga')).to.equal('/hoge/fuga');
+      done();
+    });
+
+    it('should remove trailing slash', done => {
+      expect(pathUtils.normalizePath('/hoge/fuga/')).to.equal('/hoge/fuga');
+      done();
+    });
+  });
+});