Fix sitemap 404

platform/lib/routers/pages.js

@@ -17,11 +17,11 @@
 'use strict';
 
 const express = require('express');
+const path = require('path');
 const config = require('@lib/config');
 const {Signale} = require('signale');
 const {isFilterableRoute} = require('@lib/common/filteredPage');
-const {project} = require('@lib/utils');
-
+const project = require('@lib/utils/project');
 
 // eslint-disable-next-line new-cap
 const pages = express.Router();
@@ -48,19 +48,20 @@ function getFilteredFormat(request) {
 /**
  * Checks if a path ends on a directory and appends index.html if that's
  * the case, otherwise appends .html extension
- * @param  {String} path
+ * @param  {String} filePath
  * @return {String}
  */
-function ensureFileExtension(path) {
-  if (path.endsWith('/')) {
-    return path += 'index.html';
+function ensureFileExtension(filePath) {
+  if (filePath.endsWith('/')) {
+    return filePath += 'index.html';
   }
 
-  if (!path.endsWith('.html')) {
-    return path += '.html';
+  const extension = path.extname(filePath);
+  if (!extension) {
+    return filePath += '.html';
   }
 
-  return path;
+  return filePath;
 }
 
 

platform/lib/routers/pages.test.js

@@ -0,0 +1,136 @@
+const express = require('express');
+const request = require('supertest');
+
+// We want to unit test the prod mode
+jest.setMock('@lib/config', {
+  isDevMode: () => {
+    return false;
+  },
+});
+
+const app = express();
+const router = require('./pages.js');
+app.use(router);
+
+// We use non existing urls so the static handler used by page.js will call our next router
+// The next router remembers the request, so we can then test the url rewriting
+
+let lastRequest = undefined;
+
+// eslint-disable-next-line new-cap
+const next = express.Router();
+next.get('/*', async (req, res) => {
+  lastRequest = req;
+  res.send('next');
+});
+
+app.use(next);
+
+beforeEach(() => {
+  lastRequest = undefined;
+});
+
+test('Url without extension is rewritten to .html at the end', (done) => {
+  request(app)
+      .get('/nonexisting/file')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/file.html');
+      })
+      .end(done);
+});
+
+test('Url with html extension is not rewritten to extra .html at the end', (done) => {
+  request(app)
+      .get('/nonexisting/file.html')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/file.html');
+      })
+      .end(done);
+});
+
+test('Url with xml extension is not rewritten to .html at the end', (done) => {
+  request(app)
+      .get('/nonexisting/file.xml')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/file.xml');
+      })
+      .end(done);
+});
+
+test('Url with / at the end is rewritten to index.html at the end', (done) => {
+  request(app)
+      .get('/nonexisting/')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/index.html');
+      })
+      .end(done);
+});
+
+test('Url with format website is not rewritten', (done) => {
+  request(app)
+      .get('/nonexisting/file.html?format=website')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/file.html');
+      })
+      .end(done);
+});
+
+
+test('Url with format ads is rewritten', (done) => {
+  request(app)
+      .get('/nonexisting/file.html?format=ads')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/file.ads.html');
+      })
+      .end(done);
+});
+
+test('Url with format ads is rewritten', (done) => {
+  request(app)
+      .get('/nonexisting/file.html?format=stories')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/file.stories.html');
+      })
+      .end(done);
+});
+
+test('Url with format email is rewritten', (done) => {
+  request(app)
+      .get('/nonexisting/file.html?format=email')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/file.email.html');
+      })
+      .end(done);
+});
+
+test('Url with format unknown is not rewritten', (done) => {
+  request(app)
+      .get('/nonexisting/file.html?format=unknown')
+      .expect(200, 'next')
+      .expect(() => {
+        expect(lastRequest.url).toBe('/nonexisting/file.html');
+      })
+      .end(done);
+});
+
+test('redirect index.amp.html to folder', (done) => {
+  request(app)
+      .get('/someurl/index.amp.html')
+      .expect('Location', '/someurl/')
+      .expect(302, done);
+});
+
+test('redirect file.amp.html to file without amp.html', (done) => {
+  request(app)
+      .get('/someurl/file.amp.html')
+      .expect('Location', '/someurl/file')
+      .expect(302, done);
+});

platform/lib/routers/static.js

@@ -18,6 +18,7 @@
 
 const express = require('express');
 const {setMaxAge} = require('@lib/utils/cacheHelpers');
+const {join} = require('path');
 const config = require('@lib/config');
 const project = require('@lib/utils/project');
 const robots = require('./robots');
@@ -28,7 +29,7 @@ const staticRouter = express.Router();
 staticRouter.use('/static', express.static(project.paths.STATICS_DEST));
 
 if (config.isProdMode()) {
-  staticRouter.use('/', express.static('static/sitemap'));
+  staticRouter.use('/', express.static(join(project.paths.STATICS_DEST, 'sitemap')));
 }
 
 staticRouter.get('/serviceworker.js', (request, response) => {