diff --git a/core/badge-urls/make-badge-url.js b/core/badge-urls/make-badge-url.js index f272f5ec5f..e011a3719b 100644 --- a/core/badge-urls/make-badge-url.js +++ b/core/badge-urls/make-badge-url.js @@ -9,7 +9,7 @@ function badgeUrlFromPath({ path, queryParams, style, - format = 'svg', + format = '', longCache = false, }) { const outExt = format.length ? `.${format}` : '' @@ -30,7 +30,7 @@ function badgeUrlFromPattern({ namedParams, queryParams, style, - format = 'svg', + format = '', longCache = false, }) { const toPath = pathToRegexp.compile(pattern, { @@ -61,15 +61,16 @@ function staticBadgeUrl({ color = 'lightgray', style, namedLogo, - format = 'svg', + format = '', }) { const path = [label, message, color].map(encodeField).join('-') const outQueryString = queryString.stringify({ style, logo: namedLogo, }) + const outExt = format.length ? `.${format}` : '' const suffix = outQueryString ? `?${outQueryString}` : '' - return `${baseUrl}/badge/${path}.${format}${suffix}` + return `${baseUrl}/badge/${path}${outExt}${suffix}` } function queryStringStaticBadgeUrl({ @@ -83,7 +84,7 @@ function queryStringStaticBadgeUrl({ logoColor, logoWidth, logoPosition, - format = 'svg', + format = '', }) { // schemaVersion could be a parameter if we iterate on it, // for now it's hardcoded to the only supported version. @@ -99,7 +100,8 @@ function queryStringStaticBadgeUrl({ logoWidth, logoPosition, })}` - return `${baseUrl}/static/v${schemaVersion}.${format}${suffix}` + const outExt = format.length ? `.${format}` : '' + return `${baseUrl}/static/v${schemaVersion}${outExt}${suffix}` } function dynamicBadgeUrl({ @@ -112,8 +114,10 @@ function dynamicBadgeUrl({ suffix, color, style, - format = 'svg', + format = '', }) { + const outExt = format.length ? `.${format}` : '' + const queryParams = { label, url: dataUrl, @@ -132,7 +136,7 @@ function dynamicBadgeUrl({ } const outQueryString = queryString.stringify(queryParams) - return `${baseUrl}/badge/dynamic/${datatype}.${format}?${outQueryString}` + return `${baseUrl}/badge/dynamic/${datatype}${outExt}?${outQueryString}` } function rasterRedirectUrl({ rasterUrl }, badgeUrl) { diff --git a/core/badge-urls/make-badge-url.spec.js b/core/badge-urls/make-badge-url.spec.js index 1d0f183cff..450b15d673 100644 --- a/core/badge-urls/make-badge-url.spec.js +++ b/core/badge-urls/make-badge-url.spec.js @@ -18,7 +18,7 @@ describe('Badge URL generation functions', function() { style: 'flat-square', longCache: true, }).expect( - 'http://example.com/npm/v/gh-badges.svg?cacheSeconds=2592000&style=flat-square' + 'http://example.com/npm/v/gh-badges?cacheSeconds=2592000&style=flat-square' ) }) @@ -30,7 +30,7 @@ describe('Badge URL generation functions', function() { style: 'flat-square', longCache: true, }).expect( - 'http://example.com/npm/v/gh-badges.svg?cacheSeconds=2592000&style=flat-square' + 'http://example.com/npm/v/gh-badges?cacheSeconds=2592000&style=flat-square' ) }) @@ -48,7 +48,7 @@ describe('Badge URL generation functions', function() { message: 'bar', color: 'blue', style: 'flat-square', - }).expect('/badge/foo-bar-blue.svg?style=flat-square') + }).expect('/badge/foo-bar-blue?style=flat-square') given({ label: 'foo', message: 'bar', @@ -62,24 +62,24 @@ describe('Badge URL generation functions', function() { message: 'Привет Мир', color: '#aabbcc', }).expect( - '/badge/Hello%20World-%D0%9F%D1%80%D0%B8%D0%B2%D0%B5%D1%82%20%D0%9C%D0%B8%D1%80-%23aabbcc.svg' + '/badge/Hello%20World-%D0%9F%D1%80%D0%B8%D0%B2%D0%B5%D1%82%20%D0%9C%D0%B8%D1%80-%23aabbcc' ) given({ label: '123-123', message: 'abc-abc', color: 'blue', - }).expect('/badge/123--123-abc--abc-blue.svg') + }).expect('/badge/123--123-abc--abc-blue') given({ label: '123-123', message: '', color: 'blue', style: 'social', - }).expect('/badge/123--123--blue.svg?style=social') + }).expect('/badge/123--123--blue?style=social') given({ label: '', message: 'blue', color: 'blue', - }).expect('/badge/-blue-blue.svg') + }).expect('/badge/-blue-blue') }) test(queryStringStaticBadgeUrl, () => { @@ -89,9 +89,7 @@ describe('Badge URL generation functions', function() { message: 'bar', color: 'blue', style: 'flat-square', - }).expect( - '/static/v1.svg?color=blue&label=foo&message=bar&style=flat-square' - ) + }).expect('/static/v1?color=blue&label=foo&message=bar&style=flat-square') given({ label: 'foo Bar', message: 'bar Baz', @@ -107,7 +105,7 @@ describe('Badge URL generation functions', function() { message: 'Привет Мир', color: '#aabbcc', }).expect( - '/static/v1.svg?color=%23aabbcc&label=Hello%20World&message=%D0%9F%D1%80%D0%B8%D0%B2%D0%B5%D1%82%20%D0%9C%D0%B8%D1%80' + '/static/v1?color=%23aabbcc&label=Hello%20World&message=%D0%9F%D1%80%D0%B8%D0%B2%D0%B5%D1%82%20%D0%9C%D0%B8%D1%80' ) }) @@ -125,7 +123,7 @@ describe('Badge URL generation functions', function() { style: 'plastic', }).expect( [ - 'http://img.example.com/badge/dynamic/json.svg', + 'http://img.example.com/badge/dynamic/json', '?label=foo', `&prefix=${encodeURIComponent(prefix)}`, `&query=${encodeURIComponent(query)}`, @@ -146,7 +144,7 @@ describe('Badge URL generation functions', function() { style: 'plastic', }).expect( [ - 'http://img.example.com/badge/dynamic/json.svg', + 'http://img.example.com/badge/dynamic/json', '?color=blue', '&label=foo', `&query=${encodeURIComponent(query)}`, diff --git a/core/base-service/base-non-memory-caching.js b/core/base-service/base-non-memory-caching.js index 0b442e5aa5..8aaacf3a2b 100644 --- a/core/base-service/base-non-memory-caching.js +++ b/core/base-service/base-non-memory-caching.js @@ -50,7 +50,7 @@ module.exports = class NonMemoryCachingBaseService extends BaseService { ) // The final capture group is the extension. - const format = match.slice(-1)[0] + const format = (match.slice(-1)[0] || '.svg').replace(/^\./, '') badgeData.format = format const svg = makeBadge(badgeData) diff --git a/core/base-service/base-static.js b/core/base-service/base-static.js index 3d3b48d720..7ecd598f32 100644 --- a/core/base-service/base-static.js +++ b/core/base-service/base-static.js @@ -45,7 +45,7 @@ module.exports = class BaseStaticService extends BaseService { ) // The final capture group is the extension. - const format = match.slice(-1)[0] + const format = (match.slice(-1)[0] || '.svg').replace(/^\./, '') badgeData.format = format if (shouldProfileMakeBadge) { diff --git a/core/base-service/base.js b/core/base-service/base.js index c692430dab..7ca0923cce 100644 --- a/core/base-service/base.js +++ b/core/base-service/base.js @@ -438,7 +438,7 @@ class BaseService { this ) // The final capture group is the extension. - const format = match.slice(-1)[0] + const format = (match.slice(-1)[0] || '.svg').replace(/^\./, '') sendBadge(format, badgeData) serviceRequestCounter.inc() diff --git a/core/base-service/base.spec.js b/core/base-service/base.spec.js index 7c0d68e1e2..8964cd0b27 100644 --- a/core/base-service/base.spec.js +++ b/core/base-service/base.spec.js @@ -316,7 +316,7 @@ describe('BaseService', function() { }) describe('ScoutCamp integration', function() { - const expectedRouteRegex = /^\/foo\/([^/]+?)\.(svg|json)$/ + const expectedRouteRegex = /^\/foo\/([^/]+?)(|\.svg|\.json)$/ let mockCamp let mockHandleRequest diff --git a/core/base-service/redirector.js b/core/base-service/redirector.js index d0a5d15c51..9b9acbf8d4 100644 --- a/core/base-service/redirector.js +++ b/core/base-service/redirector.js @@ -106,7 +106,7 @@ module.exports = function redirector(attrs) { } // The final capture group is the extension. - const format = match.slice(-1)[0] + const format = (match.slice(-1)[0] || '.svg').replace(/^\./, '') const redirectUrl = `${ format === 'png' ? rasterUrl : '' }${targetPath}.${format}${urlSuffix}` diff --git a/core/base-service/route.js b/core/base-service/route.js index 3bdf15de7c..d9bb8e0aca 100644 --- a/core/base-service/route.js +++ b/core/base-service/route.js @@ -1,5 +1,6 @@ 'use strict' +const escapeStringRegexp = require('escape-string-regexp') const Joi = require('@hapi/joi') const pathToRegexp = require('path-to-regexp') @@ -27,15 +28,16 @@ function assertValidRoute(route, message = undefined) { } function prepareRoute({ base, pattern, format, capture, withPng }) { - const extensionRegex = ['svg', 'json'] - .concat(withPng ? ['png'] : []) + const extensionRegex = ['', '.svg', '.json'] + .concat(withPng ? ['.png'] : []) + .map(escapeStringRegexp) .join('|') let regex, captureNames if (pattern === undefined) { - regex = new RegExp(`^${makeFullUrl(base, format)}\\.(${extensionRegex})$`) + regex = new RegExp(`^${makeFullUrl(base, format)}(${extensionRegex})$`) captureNames = capture || [] } else { - const fullPattern = `${makeFullUrl(base, pattern)}.:ext(${extensionRegex})` + const fullPattern = `${makeFullUrl(base, pattern)}:ext(${extensionRegex})` const keys = [] regex = pathToRegexp(fullPattern, keys, { strict: true, diff --git a/core/base-service/route.spec.js b/core/base-service/route.spec.js index 5b5331f8da..113162d9c9 100644 --- a/core/base-service/route.spec.js +++ b/core/base-service/route.spec.js @@ -19,13 +19,7 @@ describe('Route helpers', function() { const regexExec = str => regex.exec(str) test(regexExec, () => { - forCases([ - given('/foo/bar.bar.bar.zip'), - given('/foo/bar/bar.svg'), - // This is a valid example with the wrong extension separator, to - // test that we only accept a `.`. - given('/foo/bar.bar.bar_svg'), - ]).expect(null) + given('/foo/bar/bar.svg').expect(null) }) const namedParams = str => @@ -35,25 +29,23 @@ describe('Route helpers', function() { given('/foo/bar.bar.bar.svg'), given('/foo/bar.bar.bar.json'), ]).expect({ namedParamA: 'bar.bar.bar' }) + + // This pattern catches bugs related to escaping the extension separator. + given('/foo/bar.bar.bar_svg').expect({ namedParamA: 'bar.bar.bar_svg' }) + given('/foo/bar.bar.bar.zip').expect({ namedParamA: 'bar.bar.bar.zip' }) }) }) context('A `format` with a named param is declared', function() { const { regex, captureNames } = prepareRoute({ base: 'foo', - format: '([^/]+)', + format: '([^/]+?)', capture: ['namedParamA'], }) const regexExec = str => regex.exec(str) test(regexExec, () => { - forCases([ - given('/foo/bar.bar.bar.zip'), - given('/foo/bar/bar.svg'), - // This is a valid example with the wrong extension separator, to - // test that we only accept a `.`. - given('/foo/bar.bar.bar_svg'), - ]).expect(null) + given('/foo/bar/bar.svg').expect(null) }) const namedParams = str => @@ -63,6 +55,10 @@ describe('Route helpers', function() { given('/foo/bar.bar.bar.svg'), given('/foo/bar.bar.bar.json'), ]).expect({ namedParamA: 'bar.bar.bar' }) + + // This pattern catches bugs related to escaping the extension separator. + given('/foo/bar.bar.bar_svg').expect({ namedParamA: 'bar.bar.bar_svg' }) + given('/foo/bar.bar.bar.zip').expect({ namedParamA: 'bar.bar.bar.zip' }) }) }) diff --git a/core/server/server.js b/core/server/server.js index 048cdaf530..e038f9cdc8 100644 --- a/core/server/server.js +++ b/core/server/server.js @@ -3,7 +3,6 @@ * @module */ -const fs = require('fs') const path = require('path') const url = require('url') const bytes = require('bytes') @@ -27,11 +26,6 @@ const PrometheusMetrics = require('./prometheus-metrics') const optionalUrl = Joi.string().uri({ scheme: ['http', 'https'] }) const requiredUrl = optionalUrl.required() -const notFound = fs.readFileSync( - path.resolve(__dirname, 'error-pages', '404.html'), - 'utf-8' -) - const publicConfigSchema = Joi.object({ bind: { port: Joi.number().port(), @@ -189,7 +183,7 @@ class Server { public: { rasterUrl }, } = config - camp.notfound(/\.(gif|jpg)$/, (query, match, end, request) => { + camp.route(/\.(gif|jpg)$/, (query, match, end, request) => { const [, format] = match makeSend('svg', request.res, end)( makeBadge({ @@ -201,7 +195,7 @@ class Server { }) if (!rasterUrl) { - camp.notfound(/\.png$/, (query, match, end, request) => { + camp.route(/\.png$/, (query, match, end, request) => { makeSend('svg', request.res, end)( makeBadge({ text: ['404', 'raster badges not available'], @@ -212,8 +206,10 @@ class Server { }) } - camp.notfound(/\.(svg|json)$/, (query, match, end, request) => { - const [, format] = match + camp.notfound(/(.svg|.json|)$/, (query, match, end, request) => { + const [, extension] = match + const format = (extension || '.svg').replace(/^\./, '') + makeSend(format, request.res, end)( makeBadge({ text: ['404', 'badge not found'], @@ -222,34 +218,6 @@ class Server { }) ) }) - - camp.notfound(/.*/, (query, match, end, request) => { - end(notFound) - }) - } - - /** - * Iterate all the service classes defined in /services, - * load each service and register a Scoutcamp route for each service. - */ - registerServices() { - const { config, camp } = this - const { apiProvider: githubApiProvider } = this.githubConstellation - const { requestCounter } = this.metrics || {} - - loadServiceClasses().forEach(serviceClass => - serviceClass.register( - { camp, handleRequest, githubApiProvider, requestCounter }, - { - handleInternalErrors: config.public.handleInternalErrors, - cacheHeaders: config.public.cacheHeaders, - profiling: config.public.profiling, - fetchLimitBytes: bytes(config.public.fetchLimit), - rasterUrl: config.public.rasterUrl, - private: config.private, - } - ) - ) } /** @@ -290,6 +258,30 @@ class Server { } } + /** + * Iterate all the service classes defined in /services, + * load each service and register a Scoutcamp route for each service. + */ + registerServices() { + const { config, camp } = this + const { apiProvider: githubApiProvider } = this.githubConstellation + const { requestCounter } = this.metrics || {} + + loadServiceClasses().forEach(serviceClass => + serviceClass.register( + { camp, handleRequest, githubApiProvider, requestCounter }, + { + handleInternalErrors: config.public.handleInternalErrors, + cacheHeaders: config.public.cacheHeaders, + profiling: config.public.profiling, + fetchLimitBytes: bytes(config.public.fetchLimit), + rasterUrl: config.public.rasterUrl, + private: config.private, + } + ) + ) + } + /** * Start the HTTP server: * Bootstrap Scoutcamp, @@ -327,8 +319,8 @@ class Server { suggest.setRoutes(allowedOrigin, githubApiProvider, camp) this.registerErrorHandlers() - this.registerServices() this.registerRedirects() + this.registerServices() await new Promise(resolve => camp.on('listening', () => resolve())) } diff --git a/core/server/server.spec.js b/core/server/server.spec.js index 55916fcab7..53acc855bc 100644 --- a/core/server/server.spec.js +++ b/core/server/server.spec.js @@ -55,6 +55,15 @@ describe('The server', function() { ) }) + it('should produce json badges', async function() { + const { statusCode, body, headers } = await got( + `${baseUrl}npm/v/express.json` + ) + expect(statusCode).to.equal(200) + expect(headers['content-type']).to.equal('application/json') + expect(() => JSON.parse(body)).not.to.throw() + }) + it('should preserve label case', async function() { const { statusCode, body } = await got(`${baseUrl}:fRuiT-apple-green.svg`) expect(statusCode).to.equal(200) @@ -109,13 +118,16 @@ describe('The server', function() { .and.to.include('badge not found') }) - it('should return the 404 html page for rando links', async function() { + it('should return the 404 badge page for rando links', async function() { const { statusCode, body } = await got( `${baseUrl}this/is/most/definitely/not/a/badge.js`, { throwHttpErrors: false } ) expect(statusCode).to.equal(404) - expect(body).to.include('blood, toil, tears and sweat') + expect(body) + .to.satisfy(isSvg) + .and.to.include('404') + .and.to.include('badge not found') }) it('should redirect the root as configured', async function() { @@ -132,7 +144,8 @@ describe('The server', function() { const { statusCode, body } = await got(`${baseUrl}npm/v/express.jpg`, { throwHttpErrors: false, }) - expect(statusCode).to.equal(404) + // TODO It would be nice if this were 404 or 410. + expect(statusCode).to.equal(200) expect(body) .to.satisfy(isSvg) .and.to.include('410') diff --git a/cypress/integration/main-page.spec.js b/cypress/integration/main-page.spec.js index 0f165ef20f..31624c0cb3 100644 --- a/cypress/integration/main-page.spec.js +++ b/cypress/integration/main-page.spec.js @@ -26,13 +26,13 @@ describe('Main page', function() { expectBadgeExample( 'Discourse status', - 'http://localhost:8080/badge/discourse-online-brightgreen.svg', - '/discourse/:scheme/:host/status.svg' + 'http://localhost:8080/badge/discourse-online-brightgreen', + '/discourse/:scheme/:host/status' ) }) it('Suggest badges', function() { - const badgeUrl = `${backendUrl}/github/issues/badges/shields.svg` + const badgeUrl = `${backendUrl}/github/issues/badges/shields` cy.visit('/') cy.get(SEARCH_INPUT).type('https://github.com/badges/shields') @@ -42,7 +42,7 @@ describe('Main page', function() { }) it('Customization form is filled with suggested badge details', function() { - const badgeUrl = `${backendUrl}/github/issues/badges/shields.svg` + const badgeUrl = `${backendUrl}/github/issues/badges/shields` cy.visit('/') cy.get(SEARCH_INPUT).type('https://github.com/badges/shields') cy.contains('Suggest badges').click() @@ -54,7 +54,7 @@ describe('Main page', function() { }) it('Customizate suggested badge', function() { - const badgeUrl = `${backendUrl}/github/issues/badges/shields.svg` + const badgeUrl = `${backendUrl}/github/issues/badges/shields` cy.visit('/') cy.get(SEARCH_INPUT).type('https://github.com/badges/shields') cy.contains('Suggest badges').click() @@ -62,8 +62,6 @@ describe('Main page', function() { cy.get('table input[name="color"]').type('orange') - cy.get( - `img[src='${backendUrl}/github/issues/badges/shields.svg?color=orange']` - ) + cy.get(`img[src='${backendUrl}/github/issues/badges/shields?color=orange']`) }) }) diff --git a/doc/deprecating-badges.md b/doc/deprecating-badges.md index 9f6ad38a9c..353b56de6b 100644 --- a/doc/deprecating-badges.md +++ b/doc/deprecating-badges.md @@ -22,7 +22,7 @@ module.exports = deprecatedService({ category: 'size', route: { base: 'imagelayers', - format: '(?:.+)', + format: '(?:.+?)', }, label: 'imagelayers', dateAdded: new Date('2019-xx-xx'), // Be sure to update this with today's date! diff --git a/frontend/components/common.spec.tsx b/frontend/components/common.spec.tsx index 4fc495d30b..40f227e646 100644 --- a/frontend/components/common.spec.tsx +++ b/frontend/components/common.spec.tsx @@ -33,14 +33,12 @@ describe('Common modules', function() { describe('', function() { it('renders', function() { - shallow() + shallow() }) it('contains a link to the image', function() { - const wrapper = render() - expect(wrapper.html()).to.contain( - '' - ) + const wrapper = render() + expect(wrapper.html()).to.contain('') }) }) diff --git a/frontend/components/customizer/customizer.js b/frontend/components/customizer/customizer.js index ff4db89a4c..0321652cb9 100644 --- a/frontend/components/customizer/customizer.js +++ b/frontend/components/customizer/customizer.js @@ -36,7 +36,7 @@ export default function Customizer({ function generateBuiltBadgeUrl() { const suffix = queryString ? `?${queryString}` : '' - return `${baseUrl || getBaseUrlFromWindowLocation()}${path}.svg${suffix}` + return `${baseUrl || getBaseUrlFromWindowLocation()}${path}${suffix}` } function renderLivePreview() { diff --git a/frontend/components/snippet.spec.js b/frontend/components/snippet.spec.js index fa539614cf..09681e5f8a 100644 --- a/frontend/components/snippet.spec.js +++ b/frontend/components/snippet.spec.js @@ -6,16 +6,12 @@ import '../enzyme-conf.spec' describe('', function() { it('renders', function() { - render() + render() }) it('renders with truncate and fontSize', function() { render( - + ) }) }) diff --git a/frontend/components/usage.js b/frontend/components/usage.js index 244effaa3b..2af8fc0b36 100644 --- a/frontend/components/usage.js +++ b/frontend/components/usage.js @@ -183,13 +183,13 @@ export default function Usage({ baseUrl }) {

Using dash "-" separator

- --.svg`} /> + --`} />

Using query string parameters

&message=&color=`} + snippet={`${baseUrl}/static/v1?label=

@@ -229,7 +229,7 @@ export default function Usage({ baseUrl }) {

Endpoint

- &style