Skip to content

Commit

Permalink
Fixed mimetype check for url [ for fonts ] (#1387)
Browse files Browse the repository at this point in the history
  • Loading branch information
ninadbstack authored Oct 4, 2023
1 parent 14503d0 commit 5d95e76
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 10 deletions.
11 changes: 7 additions & 4 deletions packages/core/src/network.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import mime from 'mime-types';
import logger from '@percy/logger';
import { request as makeRequest } from '@percy/client/utils';
import { normalizeURL, hostnameMatches, createResource, waitFor } from './utils.js';
import logger from '@percy/logger';
import mime from 'mime-types';
import { createResource, hostnameMatches, normalizeURL, waitFor } from './utils.js';

const MAX_RESOURCE_SIZE = 25 * (1024 ** 2); // 25MB
const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308];
Expand Down Expand Up @@ -397,7 +397,10 @@ async function saveResponseResource(network, request) {
return log.debug(`- Skipping disallowed resource type [${request.type}]`, meta);
}

let detectedMime = mime.lookup(response.url);
// mime package does not handle query params
let urlObj = new URL(url);
let urlWithoutSearchParams = urlObj.origin + urlObj.pathname;
let detectedMime = mime.lookup(urlWithoutSearchParams);
let mimeType = (
// ensure the mimetype is correct for text/plain responses
response.mimeType === 'text/plain' && detectedMime
Expand Down
10 changes: 5 additions & 5 deletions packages/core/test/discovery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,11 +384,11 @@ describe('Discovery', () => {
it('fetches font file correctly with makeDirect', async () => {
// add font to page via stylesheet
server.reply('/style.css', () => [200, 'text/css', [
'@font-face { font-family: "test"; src: url("/font.woff") format("woff"); }',
'@font-face { font-family: "test"; src: url("/font.woff?abc=1") format("woff"); }',
'body { font-family: "test", "sans-serif"; }'
].join('')]);

server.reply('/font.woff', () => {
server.reply('/font.woff?abc=1', () => {
return [200, 'application/octate-stream', '<font>'];
});

Expand All @@ -401,17 +401,17 @@ describe('Discovery', () => {
await percy.idle();
// confirm that request was made 2 times, once via browser and once due to makeDirectRequest
let paths = server.requests.map(r => r[0]);
expect(paths.filter(x => x === '/font.woff').length).toEqual(2);
expect(paths.filter(x => x === '/font.woff?abc=1').length).toEqual(2);

let requestData = captured[0].map((x) => x.attributes)
.filter(x => x['resource-url'] === 'http://localhost:8000/font.woff')[0];
.filter(x => x['resource-url'] === 'http://localhost:8000/font.woff?abc=1')[0];

// confirm that original response mimetype is not tampered
expect(requestData.mimetype).toEqual('application/octate-stream');
});
});

it('does not mimetype parse resourced with no file extension', async () => {
it('does not mimetype parse resource with no file extension', async () => {
let brokeDOM = testDOM.replace('style.css', 'broken-css');
server.reply('/broken-css', () => [200, 'text/plain', testCSS]);
percy.loglevel('debug');
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/helpers/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function createTestServer({ default: defaultReply, ...replies }, port = 8
let pathname = req.url.pathname;
if (req.url.search) pathname += req.url.search;
server.requests.push(req.body ? [pathname, req.body] : [pathname]);
let reply = replies[req.url.pathname] || defaultReply;
let reply = replies[pathname] || defaultReply;
return reply ? await reply(req, res) : next();
});

Expand Down

0 comments on commit 5d95e76

Please sign in to comment.