Fix & refactor server routing + add tests (#799)

* Fix bad routing regex for sitemap & feed

* add tests for sitemap & feed

* use next middleware function if file nto found

* add pages routing & test

* refactor + add more test for page routing

* extension-less url routing + test

* refactor out requestFile

* add dot routing + test to handle special case like http://localhost:3000/blog/2018/05/27/1.13.0

* exit properly

* add more test for sitemap

* update nits from my phone

lib/core/__tests__/routing.test.js

@@ -5,7 +5,15 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-const {docsRouting, blogRouting} = require('../routing');
+const {
+  blogRouting,
+  docsRouting,
+  dotRouting,
+  feedRouting,
+  noExtRouting,
+  pageRouting,
+  sitemapRouting,
+} = require('../routing');
 
 describe('Blog routing', () => {
   const blogRegex = blogRouting('/');
@@ -60,3 +68,129 @@ describe('Docs routing', () => {
     expect('/reason/blog/docs/docs.html').not.toMatch(docsRegex2);
   });
 });
+
+describe('Dot routing', () => {
+  const dotRegex = dotRouting();
+
+  test('valid url with dot after last slash', () => {
+    expect('/docs/en/test.23').toMatch(dotRegex);
+    expect('/robots.hai.2').toMatch(dotRegex);
+    expect('/blog/1.2.3').toMatch(dotRegex);
+    expect('/this.is.my').toMatch(dotRegex);
+  });
+
+  test('html file is invalid', () => {
+    expect('/docs/en.html').not.toMatch(dotRegex);
+    expect('/users.html').not.toMatch(dotRegex);
+    expect('/blog/asdf.html').not.toMatch(dotRegex);
+    expect('/end/1234/asdf.html').not.toMatch(dotRegex);
+    expect('/test/lol.huam.html').not.toMatch(dotRegex);
+  });
+
+  test('extension-less url is not valid', () => {
+    expect('/reason/test').not.toMatch(dotRegex);
+    expect('/asdff').not.toMatch(dotRegex);
+    expect('/blog/asdf.ghg/').not.toMatch(dotRegex);
+    expect('/end/1234.23.55/').not.toMatch(dotRegex);
+  });
+});
+
+describe('Feed routing', () => {
+  const feedRegex = feedRouting('/');
+  const feedRegex2 = feedRouting('/reason/');
+
+  test('valid feed url', () => {
+    expect('/blog/atom.xml').toMatch(feedRegex);
+    expect('/blog/feed.xml').toMatch(feedRegex);
+    expect('/reason/blog/atom.xml').toMatch(feedRegex2);
+    expect('/reason/blog/feed.xml').toMatch(feedRegex2);
+  });
+
+  test('invalid feed url', () => {
+    expect('/blog/blog/feed.xml').not.toMatch(feedRegex);
+    expect('/blog/test.xml').not.toMatch(feedRegex);
+    expect('/reason/blog/atom.xml').not.toMatch(feedRegex);
+    expect('/reason/blog/feed.xml').not.toMatch(feedRegex);
+    expect('/blog/feed.xml/test.html').not.toMatch(feedRegex);
+    expect('/blog/atom.xml').not.toMatch(feedRegex2);
+    expect('/blog/feed.xml').not.toMatch(feedRegex2);
+    expect('/reason/blog/test.xml').not.toMatch(feedRegex2);
+    expect('/reason/blog/blog/feed.xml').not.toMatch(feedRegex2);
+    expect('/reason/blog/blog/atom.xml').not.toMatch(feedRegex2);
+  });
+
+  test('not a feed', () => {
+    expect('/blog/atom').not.toMatch(feedRegex);
+    expect('/reason/blog/feed').not.toMatch(feedRegex2);
+  });
+});
+
+describe('Extension-less url routing', () => {
+  const noExtRegex = noExtRouting();
+
+  test('valid no extension url', () => {
+    expect('/test').toMatch(noExtRegex);
+    expect('/reason/test').toMatch(noExtRegex);
+  });
+
+  test('url with file extension', () => {
+    expect('/robots.txt').not.toMatch(noExtRegex);
+    expect('/reason/robots.txt').not.toMatch(noExtRegex);
+    expect('/docs/en/docu.html').not.toMatch(noExtRegex);
+    expect('/reason/robots.html').not.toMatch(noExtRegex);
+    expect('/blog/atom.xml').not.toMatch(noExtRegex);
+    expect('/reason/sitemap.xml').not.toMatch(noExtRegex);
+    expect('/main.css').not.toMatch(noExtRegex);
+    expect('/reason/custom.css').not.toMatch(noExtRegex);
+  });
+});
+
+describe('Page routing', () => {
+  const pageRegex = pageRouting('/');
+  const pageRegex2 = pageRouting('/reason/');
+
+  test('valid page url', () => {
+    expect('/index.html').toMatch(pageRegex);
+    expect('/en/help.html').toMatch(pageRegex);
+    expect('/reason/index.html').toMatch(pageRegex2);
+    expect('/reason/ro/users.html').toMatch(pageRegex2);
+  });
+
+  test('docs not considered as page', () => {
+    expect('/docs/en/test.html').not.toMatch(pageRegex);
+    expect('/reason/docs/en/test.html').not.toMatch(pageRegex2);
+  });
+
+  test('blog not considered as page', () => {
+    expect('/blog/index.html').not.toMatch(pageRegex);
+    expect('/reason/blog/index.html').not.toMatch(pageRegex2);
+  });
+
+  test('not a page', () => {
+    expect('/yangshun.jpg').not.toMatch(pageRegex);
+    expect('/reason/endilie.png').not.toMatch(pageRegex2);
+  });
+});
+
+describe('Sitemap routing', () => {
+  const sitemapRegex = sitemapRouting('/');
+  const sitemapRegex2 = sitemapRouting('/reason/');
+
+  test('valid sitemap url', () => {
+    expect('/sitemap.xml').toMatch(sitemapRegex);
+    expect('/reason/sitemap.xml').toMatch(sitemapRegex2);
+  });
+
+  test('invalid sitemap url', () => {
+    expect('/reason/sitemap.xml').not.toMatch(sitemapRegex);
+    expect('/reason/sitemap.xml.html').not.toMatch(sitemapRegex);
+    expect('/sitemap/sitemap.xml').not.toMatch(sitemapRegex);
+    expect('/reason/sitemap/sitemap.xml').not.toMatch(sitemapRegex);
+    expect('/sitemap.xml').not.toMatch(sitemapRegex2);
+  });
+
+  test('not a sitemap', () => {
+    expect('/sitemap').not.toMatch(sitemapRegex);
+    expect('/reason/sitemap').not.toMatch(sitemapRegex2);
+  });
+});

lib/core/routing.js

@@ -4,17 +4,47 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  */
-const escapeStringRegexp = require('escape-string-regexp');
+const escape = require('escape-string-regexp');
-
-function docsRouting(baseUrl) {
-  return new RegExp(`^${escapeStringRegexp(baseUrl)}docs\/.*html$`);
-}
 
 function blogRouting(baseUrl) {
-  return new RegExp(`^${escapeStringRegexp(baseUrl)}blog\/.*html$`);
+  return new RegExp(`^${escape(baseUrl)}blog\/.*html$`);
+}
+
+function docsRouting(baseUrl) {
+  return new RegExp(`^${escape(baseUrl)}docs\/.*html$`);
+}
+
+function dotRouting() {
+  return /(?!.*html$)^\/.*\.[^\n\/]+$/;
+}
+
+function feedRouting(baseUrl) {
+  return new RegExp(`^${escape(baseUrl)}blog\/(feed\.xml|atom\.xml)$`);
+}
+
+function noExtRouting() {
+  return /\/[^\.]*\/?$/;
+}
+
+function pageRouting(baseUrl) {
+  const gr = regex => regex.toString().replace(/(^\/|\/$)/gm, '');
+  return new RegExp(
+    `(?!${gr(docsRouting(baseUrl))}|${gr(blogRouting(baseUrl))})^${escape(
+      baseUrl
+    )}.*\.html$`
+  );
+}
+
+function sitemapRouting(baseUrl) {
+  return new RegExp(`^${escape(baseUrl)}sitemap.xml$`);
 }
 
 module.exports = {
-  docsRouting,
   blogRouting,
+  docsRouting,
+  dotRouting,
+  feedRouting,
+  pageRouting,
+  noExtRouting,
+  sitemapRouting,
 };

lib/server/server.js

@@ -20,7 +20,15 @@ function execute(port, options) {
   const path = require('path');
   const color = require('color');
   const getTOC = require('../core/getTOC');
-  const {docsRouting, blogRouting} = require('../core/routing');
+  const {
+    blogRouting,
+    docsRouting,
+    dotRouting,
+    feedRouting,
+    pageRouting,
+    noExtRouting,
+    sitemapRouting,
+  } = require('../core/routing');
   const mkdirp = require('mkdirp');
   const glob = require('glob');
   const chalk = require('chalk');
@@ -123,6 +131,24 @@ function execute(port, options) {
     return false;
   }
 
+  function requestFile(url, res, notFoundCallback) {
+    request.get(url, (error, response, body) => {
+      if (!error) {
+        if (response) {
+          if (response.statusCode === 404 && notFoundCallback) {
+            notFoundCallback();
+          } else {
+            res.status(response.statusCode).send(body);
+          }
+        } else {
+          console.error('No response');
+        }
+      } else {
+        console.error('Request failed:', error);
+      }
+    });
+  }
+
   /****************************************************************************/
 
   reloadMetadata();
@@ -257,7 +283,7 @@ function execute(port, options) {
     res.send(renderToStaticMarkupWithDoctype(docComp));
   });
 
-  app.get('/sitemap.xml', function(req, res) {
+  app.get(sitemapRouting(siteConfig.baseUrl), (req, res) => {
     res.set('Content-Type', 'application/xml');
 
     sitemap(xml => {
@@ -265,18 +291,22 @@ function execute(port, options) {
     });
   });
 
-  app.get(/blog\/.*xml$/, (req, res) => {
+  app.get(feedRouting(siteConfig.baseUrl), (req, res, next) => {
     res.set('Content-Type', 'application/rss+xml');
-    let parts = req.path.toString().split('blog/');
+    let file = req.path
-    if (parts[1].toLowerCase() == 'atom.xml') {
+      .toString()
+      .split('blog/')[1]
+      .toLowerCase();
+    if (file === 'atom.xml') {
       res.send(feed('atom'));
-      return;
+    } else if (file === 'feed.xml') {
+      res.send(feed('rss'));
     }
-    res.send(feed('rss'));
+    next();
   });
 
   // Handle all requests for blog pages and posts.
-  app.get(blogRouting(siteConfig.baseUrl), (req, res) => {
+  app.get(blogRouting(siteConfig.baseUrl), (req, res, next) => {
     // Regenerate the blog metadata in case it has changed. Consider improving
     // this to regenerate on file save rather than on page request.
     reloadMetadataBlog();
@@ -330,6 +360,11 @@ function execute(port, options) {
       file = file.replace(new RegExp('/', 'g'), '-');
       file = join(CWD, 'blog', file);
 
+      if (!fs.existsSync(file)) {
+        next();
+        return;
+      }
+
       const result = metadataUtils.extractMetadata(
         fs.readFileSync(file, {encoding: 'utf8'})
       );
@@ -361,7 +396,7 @@ function execute(port, options) {
   });
 
   // handle all other main pages
-  app.get('*.html', (req, res, next) => {
+  app.get(pageRouting(siteConfig.baseUrl), (req, res, next) => {
     // look for user provided html file first
     let htmlFile = req.path.toString().replace(siteConfig.baseUrl, '');
     htmlFile = join(CWD, 'pages', htmlFile);
@@ -394,6 +429,7 @@ function execute(port, options) {
       } else {
         res.send(fs.readFileSync(htmlFile, {encoding: 'utf8'}));
       }
+      next();
       return;
     }
 
@@ -535,36 +571,30 @@ function execute(port, options) {
 
   // "redirect" requests to pages ending with "/" or no extension so that,
   // for example, request to "blog" returns "blog/index.html" or "blog.html"
-  app.get(/\/[^\.]*\/?$/, (req, res) => {
+  app.get(noExtRouting(), (req, res, next) => {
-    const requestFile = (url, notFoundCallback) => {
-      request.get(url, (error, response, body) => {
-        if (!error) {
-          if (response) {
-            if (response.statusCode === 404 && notFoundCallback) {
-              notFoundCallback();
-            } else {
-              res.status(response.statusCode).send(body);
-            }
-          } else {
-            console.error('No response');
-          }
-        } else {
-          console.error('Request failed:', error);
-        }
-      });
-    };
     let slash = req.path.toString().endsWith('/') ? '' : '/';
     let requestUrl = 'http://localhost:' + port + req.path;
-    requestFile(requestUrl + slash + 'index.html', () => {
+    requestFile(requestUrl + slash + 'index.html', res, () => {
       requestFile(
         slash === '/'
           ? requestUrl + '.html'
           : requestUrl.replace(/\/$/, '.html'),
-        null
+        res,
+        next
       );
     });
   });
 
+  // handle special cleanUrl case like '/blog/1.2.3' & '/blog.robots.hai'
+  // where we should try to serve 'blog/1.2.3.html' & '/blog.robots.hai.html'
+  app.get(dotRouting(), (req, res, next) => {
+    if (!siteConfig.cleanUrl) {
+      next();
+      return;
+    }
+    requestFile('http://localhost:' + port + req.path + '.html', res, next);
+  });
+
   if (options.watch) startLiveReload();
   app.listen(port);