Merge pull request #3843 from weseek/fix/6034-6036-adjust-create-page-path

Fix/6034 6036 adjust create page path

src/client/js/components/PageCreateModal.jsx

@@ -8,7 +8,7 @@ import { withTranslation } from 'react-i18next';
 import { format } from 'date-fns';
 import urljoin from 'url-join';
 
-import { userPageRoot } from '@commons/util/path-utils';
+import { userPageRoot, isCreatablePage } from '@commons/util/path-utils';
 import { pathUtils } from 'growi-commons';
 
 import AppContainer from '../services/AppContainer';
@@ -24,12 +24,12 @@ const PageCreateModal = (props) => {
   const isReachable = config.isSearchServiceReachable;
   const pathname = decodeURI(window.location.pathname);
   const userPageRootPath = userPageRoot(appContainer.currentUser);
-  const parentPath = pathUtils.addTrailingSlash(pathname);
+  const pageNameInputInitialValue = isCreatablePage(pathname) ? pathUtils.addTrailingSlash(pathname) : '/';
   const now = format(new Date(), 'yyyy/MM/dd');
 
   const [todayInput1, setTodayInput1] = useState(t('Memo'));
   const [todayInput2, setTodayInput2] = useState('');
-  const [pageNameInput, setPageNameInput] = useState(parentPath);
+  const [pageNameInput, setPageNameInput] = useState(pageNameInputInitialValue);
   const [template, setTemplate] = useState(null);
 
   function transitBySubmitEvent(e, transitHandler) {
@@ -162,7 +162,7 @@ const PageCreateModal = (props) => {
               {isReachable
                 ? (
                   <PagePathAutoComplete
-                    initializedPath={pathname}
+                    initializedPath={pageNameInput}
                     addTrailingSlash
                     onSubmit={ppacSubmitHandler}
                     onInputChange={ppacInputChangeHandler}

src/lib/util/path-utils.js

@@ -37,6 +37,38 @@ const isUserPage = (path) => {
   return false;
 };
 
+const forbiddenPages = [
+  /\^|\$|\*|\+|#|%/,
+  /^\/-\/.*/,
+  /^\/_r\/.*/,
+  /^\/_apix?(\/.*)?/,
+  /^\/?https?:\/\/.+$/, // avoid miss in renaming
+  /\/{2,}/, // avoid miss in renaming
+  /\s+\/\s+/, // avoid miss in renaming
+  /.+\/edit$/,
+  /.+\.md$/,
+  /^(\.\.)$/, // see: https://github.com/weseek/growi/issues/3582
+  /(\/\.\.)\/?/, // see: https://github.com/weseek/growi/issues/3582
+  /^\/(installer|register|login|logout|admin|me|files|trash|paste|comments|tags|share)(\/.*|$)/,
+];
+
+/**
+ * Whether path can be created
+ * @param {string} path
+ * @returns {boolean}
+ */
+const isCreatablePage = (path) => {
+  let isCreatable = true;
+  forbiddenPages.forEach((page) => {
+    const pageNameReg = new RegExp(page);
+    if (path.match(pageNameReg)) {
+      isCreatable = false;
+    }
+  });
+
+  return isCreatable;
+};
+
 /**
  * return user path
  * @param {Object} user
@@ -83,6 +115,7 @@ module.exports = {
   isTopPage,
   isTrashPage,
   isUserPage,
+  isCreatablePage,
   userPageRoot,
   convertToNewAffiliationPath,
   encodeSpaces,

src/server/models/page.js

@@ -522,33 +522,6 @@ module.exports = function(crowi) {
     return true;
   };
 
-  pageSchema.statics.isCreatableName = function(name) {
-    const forbiddenPages = [
-      /\^|\$|\*|\+|#|%/,
-      /^\/-\/.*/,
-      /^\/_r\/.*/,
-      /^\/_apix?(\/.*)?/,
-      /^\/?https?:\/\/.+$/, // avoid miss in renaming
-      /\/{2,}/, // avoid miss in renaming
-      /\s+\/\s+/, // avoid miss in renaming
-      /.+\/edit$/,
-      /.+\.md$/,
-      /^(\.\.)$/, // see: https://github.com/weseek/growi/issues/3582
-      /(\/\.\.)\/?/, // see: https://github.com/weseek/growi/issues/3582
-      /^\/(installer|register|login|logout|admin|me|files|trash|paste|comments|tags|share)(\/.*|$)/,
-    ];
-
-    let isCreatable = true;
-    forbiddenPages.forEach((page) => {
-      const pageNameReg = new RegExp(page);
-      if (name.match(pageNameReg)) {
-        isCreatable = false;
-      }
-    });
-
-    return isCreatable;
-  };
-
   pageSchema.statics.fixToCreatableName = function(path) {
     return path
       .replace(/\/\//g, '/');

src/server/routes/apiv3/pages.js

@@ -6,6 +6,7 @@ const pathUtils = require('growi-commons').pathUtils;
 
 const { body } = require('express-validator');
 const { query } = require('express-validator');
+const { isCreatablePage } = require('@commons/util/path-utils');
 const ErrorV3 = require('../../models/vo/error-apiv3');
 
 const router = express.Router();
@@ -378,7 +379,7 @@ module.exports = (crowi) => {
       socketClientId: +req.body.socketClientId || undefined,
     };
 
-    if (!Page.isCreatableName(newPagePath)) {
+    if (!isCreatablePage(newPagePath)) {
       return res.apiv3Err(new ErrorV3(`Could not use the path '${newPagePath})'`, 'invalid_path'), 409);
     }
 

src/server/routes/page.js

@@ -1,3 +1,4 @@
+const { isCreatablePage } = require('@commons/util/path-utils');
 const { serializePageSecurely } = require('../models/serializers/page-serializer');
 const { serializeRevisionSecurely } = require('../models/serializers/revision-serializer');
 const { serializeUserSecurely } = require('../models/serializers/user-serializer');
@@ -478,12 +479,10 @@ module.exports = function(crowi, app) {
   actions.notFound = async function(req, res) {
     const path = getPathFromRequest(req);
 
-    const isCreatable = Page.isCreatableName(path);
-
     let view;
     const renderVars = { path };
 
-    if (!isCreatable) {
+    if (!isCreatablePage(path)) {
       view = 'layout-growi/not_creatable';
     }
     else if (req.isForbidden) {
@@ -1231,124 +1230,6 @@ module.exports = function(crowi, app) {
     return res.json(ApiResponse.success(result));
   };
 
-  /**
-   * @swagger
-   *
-   *    /pages.rename:
-   *      post:
-   *        tags: [Pages, CrowiCompatibles]
-   *        operationId: renamePage
-   *        summary: /pages.rename
-   *        description: Rename page
-   *        requestBody:
-   *          content:
-   *            application/json:
-   *              schema:
-   *                properties:
-   *                  page_id:
-   *                    $ref: '#/components/schemas/Page/properties/_id'
-   *                  path:
-   *                    $ref: '#/components/schemas/Page/properties/path'
-   *                  revision_id:
-   *                    $ref: '#/components/schemas/Revision/properties/_id'
-   *                  new_path:
-   *                    type: string
-   *                    description: new path
-   *                    example: /user/alice/new_test
-   *                  create_redirect:
-   *                    type: boolean
-   *                    description: whether redirect page
-   *                required:
-   *                  - page_id
-   *        responses:
-   *          200:
-   *            description: Succeeded to rename page.
-   *            content:
-   *              application/json:
-   *                schema:
-   *                  properties:
-   *                    ok:
-   *                      $ref: '#/components/schemas/V1Response/properties/ok'
-   *                    page:
-   *                      $ref: '#/components/schemas/Page'
-   *          403:
-   *            $ref: '#/components/responses/403'
-   *          500:
-   *            $ref: '#/components/responses/500'
-   */
-  /**
-   * @api {post} /pages.rename Rename page
-   * @apiName RenamePage
-   * @apiGroup Page
-   *
-   * @apiParam {String} page_id Page Id.
-   * @apiParam {String} path
-   * @apiParam {String} revision_id
-   * @apiParam {String} new_path New path name.
-   * @apiParam {Bool} create_redirect
-   */
-  api.rename = async function(req, res) {
-    const pageId = req.body.page_id;
-    const previousRevision = req.body.revision_id || null;
-    let newPagePath = pathUtils.normalizePath(req.body.new_path);
-    const options = {
-      createRedirectPage: (req.body.create_redirect != null),
-      updateMetadata: (req.body.remain_metadata == null),
-      socketClientId: +req.body.socketClientId || undefined,
-    };
-    const isRecursively = (req.body.recursively != null);
-
-    if (!Page.isCreatableName(newPagePath)) {
-      return res.json(ApiResponse.error(`Could not use the path '${newPagePath})'`, 'invalid_path'));
-    }
-
-    // check whether path starts slash
-    newPagePath = pathUtils.addHeadingSlash(newPagePath);
-
-    const isExist = await Page.count({ path: newPagePath }) > 0;
-    if (isExist) {
-      // if page found, cannot cannot rename to that path
-      return res.json(ApiResponse.error(`'new_path=${newPagePath}' already exists`, 'already_exists'));
-    }
-
-    let page;
-
-    try {
-      page = await Page.findByIdAndViewer(pageId, req.user);
-
-      if (page == null) {
-        return res.json(ApiResponse.error(`Page '${pageId}' is not found or forbidden`, 'notfound_or_forbidden'));
-      }
-
-      if (!page.isUpdatable(previousRevision)) {
-        return res.json(ApiResponse.error('Someone could update this page, so couldn\'t delete.', 'outdated'));
-      }
-
-      page = await crowi.pageService.renamePage(page, newPagePath, req.user, options, isRecursively);
-    }
-    catch (err) {
-      logger.error(err);
-      return res.json(ApiResponse.error('Failed to update page.', 'unknown'));
-    }
-
-    const result = {};
-    result.page = page; // TODO consider to use serializePageSecurely method -- 2018.08.06 Yuki Takei
-
-    res.json(ApiResponse.success(result));
-
-    try {
-      // global notification
-      await globalNotificationService.fire(GlobalNotificationSetting.EVENT.PAGE_MOVE, page, req.user, {
-        oldPath: req.body.path,
-      });
-    }
-    catch (err) {
-      logger.error('Move notification failed', err);
-    }
-
-    return page;
-  };
-
   /**
    * @swagger
    *

src/server/util/createGrowiPagesFromImports.js

@@ -1,3 +1,5 @@
+const { isCreatablePage } = require('@commons/util/path-utils');
+
 module.exports = (crowi) => {
   const Page = crowi.model('Page');
 
@@ -18,7 +20,7 @@ module.exports = (crowi) => {
       const path = page.path;
       const user = page.user;
       const body = page.body;
-      const isCreatableName = await Page.isCreatableName(path);
+      const isCreatableName = isCreatablePage(path);
       const isPageNameTaken = await Page.findByPathAndViewer(path, user);
 
       if (isCreatableName && !isPageNameTaken) {

src/test/models/page.test.js

@@ -168,56 +168,6 @@ describe('Page', () => {
     });
   });
 
-  describe('.isCreatableName', () => {
-    test('should decide creatable or not', () => {
-      expect(Page.isCreatableName('/hoge')).toBeTruthy();
-
-      // edge cases
-      expect(Page.isCreatableName('/me')).toBeFalsy();
-      expect(Page.isCreatableName('/me/')).toBeFalsy();
-      expect(Page.isCreatableName('/me/x')).toBeFalsy();
-      expect(Page.isCreatableName('/meeting')).toBeTruthy();
-      expect(Page.isCreatableName('/meeting/x')).toBeTruthy();
-
-      // end with "edit"
-      expect(Page.isCreatableName('/meeting/edit')).toBeFalsy();
-
-      // under score
-      expect(Page.isCreatableName('/_')).toBeTruthy();
-      expect(Page.isCreatableName('/_template')).toBeTruthy();
-      expect(Page.isCreatableName('/__template')).toBeTruthy();
-      expect(Page.isCreatableName('/_r/x')).toBeFalsy();
-      expect(Page.isCreatableName('/_api')).toBeFalsy();
-      expect(Page.isCreatableName('/_apix')).toBeFalsy();
-      expect(Page.isCreatableName('/_api/x')).toBeFalsy();
-
-      expect(Page.isCreatableName('/hoge/xx.md')).toBeFalsy();
-
-      // relative path
-      expect(Page.isCreatableName('/..')).toBeFalsy();
-      expect(Page.isCreatableName('/../page')).toBeFalsy();
-      expect(Page.isCreatableName('/page/..')).toBeFalsy();
-      expect(Page.isCreatableName('/page/../page')).toBeFalsy();
-
-      // start with https?
-      expect(Page.isCreatableName('/http://demo.growi.org/hoge')).toBeFalsy();
-      expect(Page.isCreatableName('/https://demo.growi.org/hoge')).toBeFalsy();
-      expect(Page.isCreatableName('http://demo.growi.org/hoge')).toBeFalsy();
-      expect(Page.isCreatableName('https://demo.growi.org/hoge')).toBeFalsy();
-
-      expect(Page.isCreatableName('/ the / path / with / space')).toBeFalsy();
-
-      const forbidden = ['installer', 'register', 'login', 'logout',
-                         'admin', 'files', 'trash', 'paste', 'comments'];
-      for (let i = 0; i < forbidden.length; i++) {
-        const pn = forbidden[i];
-        expect(Page.isCreatableName(`/${pn}`)).toBeFalsy();
-        expect(Page.isCreatableName(`/${pn}/`)).toBeFalsy();
-        expect(Page.isCreatableName(`/${pn}/abc`)).toBeFalsy();
-      }
-    });
-  });
-
   describe('.isAccessiblePageByViewer', () => {
     describe('with a granted page', () => {
       test('should return true with granted user', async() => {

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

@@ -1,4 +1,4 @@
-const { isTopPage, convertToNewAffiliationPath } = require('../../lib/util/path-utils');
+const { isTopPage, convertToNewAffiliationPath, isCreatablePage } = require('../../lib/util/path-utils');
 
 
 describe('TopPage Path test', () => {
@@ -55,3 +55,53 @@ describe('convertToNewAffiliationPath test', () => {
     }).toThrow();
   });
 });
+
+describe('isCreatablePage test', () => {
+  test('should decide creatable or not', () => {
+    expect(isCreatablePage('/hoge')).toBeTruthy();
+
+    // edge cases
+    expect(isCreatablePage('/me')).toBeFalsy();
+    expect(isCreatablePage('/me/')).toBeFalsy();
+    expect(isCreatablePage('/me/x')).toBeFalsy();
+    expect(isCreatablePage('/meeting')).toBeTruthy();
+    expect(isCreatablePage('/meeting/x')).toBeTruthy();
+
+    // end with "edit"
+    expect(isCreatablePage('/meeting/edit')).toBeFalsy();
+
+    // under score
+    expect(isCreatablePage('/_')).toBeTruthy();
+    expect(isCreatablePage('/_template')).toBeTruthy();
+    expect(isCreatablePage('/__template')).toBeTruthy();
+    expect(isCreatablePage('/_r/x')).toBeFalsy();
+    expect(isCreatablePage('/_api')).toBeFalsy();
+    expect(isCreatablePage('/_apix')).toBeFalsy();
+    expect(isCreatablePage('/_api/x')).toBeFalsy();
+
+    expect(isCreatablePage('/hoge/xx.md')).toBeFalsy();
+
+    // relative path
+    expect(isCreatablePage('/..')).toBeFalsy();
+    expect(isCreatablePage('/../page')).toBeFalsy();
+    expect(isCreatablePage('/page/..')).toBeFalsy();
+    expect(isCreatablePage('/page/../page')).toBeFalsy();
+
+    // start with https?
+    expect(isCreatablePage('/http://demo.growi.org/hoge')).toBeFalsy();
+    expect(isCreatablePage('/https://demo.growi.org/hoge')).toBeFalsy();
+    expect(isCreatablePage('http://demo.growi.org/hoge')).toBeFalsy();
+    expect(isCreatablePage('https://demo.growi.org/hoge')).toBeFalsy();
+
+    expect(isCreatablePage('/ the / path / with / space')).toBeFalsy();
+
+    const forbidden = ['installer', 'register', 'login', 'logout',
+                       'admin', 'files', 'trash', 'paste', 'comments'];
+    for (let i = 0; i < forbidden.length; i++) {
+      const pn = forbidden[i];
+      expect(isCreatablePage(`/${pn}`)).toBeFalsy();
+      expect(isCreatablePage(`/${pn}/`)).toBeFalsy();
+      expect(isCreatablePage(`/${pn}/abc`)).toBeFalsy();
+    }
+  });
+});