Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sitemap 404 #2262

Merged
merged 2 commits into from Jun 7, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions platform/lib/routers/pages.js
Expand Up @@ -20,8 +20,9 @@ const express = require('express');
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');

const PATH_HAS_FILE_EXTENSION_PATTERN = /\/[^\/]+\.[a-z0-9]{1,5}$/i;

// eslint-disable-next-line new-cap
const pages = express.Router();
Expand Down Expand Up @@ -56,7 +57,7 @@ function ensureFileExtension(path) {
return path += 'index.html';
}

if (!path.endsWith('.html')) {
if (!PATH_HAS_FILE_EXTENSION_PATTERN.test(path)) {
tharders marked this conversation as resolved.
Show resolved Hide resolved
return path += '.html';
}

Expand Down
146 changes: 146 additions & 0 deletions platform/lib/routers/pages.test.js
@@ -0,0 +1,146 @@
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 dot but no real extension is rewritten to .html at the end', (done) => {
request(app)
.get('/nonexisting/file.foo_bar')
.expect(200, 'next')
.expect(() => {
expect(lastRequest.url).toBe('/nonexisting/file.foo_bar.html');
})
.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);
});
3 changes: 2 additions & 1 deletion platform/lib/routers/static.js
Expand Up @@ -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');
Expand All @@ -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) => {
Expand Down