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

🐛 Fixed mimetype check for url [ for fonts ] #1387

Merged
merged 1 commit into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading