Skip to content

Commit

Permalink
Add support for rawContent to CDP collector
Browse files Browse the repository at this point in the history 
Work on #164
Close #253
  • Loading branch information
@alrra
Anton Molleda authored and alrra committed Jun 9, 2017
1 parent c373da9 commit 61ca59e
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 61 deletions.
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"browserslist": "^2.1.4",
"canvas-prebuilt": "^1.6.0-canvas.5",
"chalk": "^1.1.3",
"chrome-remote-interface": "^0.23.0",
"chrome-remote-interface": "^0.23.2",
"content-type": "^1.0.2",
"debug": "^2.6.8",
"eslint": "^3.19.0",
Expand Down Expand Up @@ -134,7 +134,7 @@
"lint:md": "markdownlint ./.github CHANGELOG.md CODE_OF_CONDUCT.md README.md docs",
"site": "node dist/src/bin/sonar",
"test": "npm run lint && npm run build && nyc ava",
"test-on-travis": "npm run lint && npm run build && nyc ava \"dist/tests/**/*.js\" --concurrency=1 --timeout=2m",
"test-on-travis": "npm run lint && npm run build && nyc ava \"dist/tests/**/*.js\" --concurrency=2 --timeout=2m",
"watch": "npm run build && npm-run-all --parallel -c watch:*",
"watch:resources": "npm run build:assets -- -w --no-initial",
"watch:test": "ava --watch",
Expand Down
75 changes: 54 additions & 21 deletions src/lib/collectors/cdp/cdp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class CDPCollector implements ICollector {

constructor(server: Sonar, config: object) {
const defaultOptions = {
loadCompleteRetryInterval: 500,
loadCompleteRetryInterval: 250,
maxLoadWaitTime: 30000,
waitFor: 5000
};
Expand Down Expand Up @@ -226,25 +226,30 @@ class CDPCollector implements ICollector {
await this._server.emitAsync(eventName, event);
}

/** Event handler fired when HTTP response is available and DOM loaded. */
private async onResponseReceived(params) {
// DOM is not ready so we queue up the event for later
if (!this._dom) {
this._pendingResponseReceived.push(this.onResponseReceived.bind(this, params));
private async getResponseBody(cdpResponse) {
let content = '';
let rawContent = null;
let rawResponse = null;

return;
if (cdpResponse.response.status !== 200) {
return { content, rawContent, rawResponse };
}

const resourceUrl = params.response.url;
const resourceHeaders = normalizeHeaders(params.response.headers);
let resourceBody = '';
const hops = this._redirects.calculate(resourceUrl);
const originalUrl = hops[0] || resourceUrl;

try {
resourceBody = (await this._client.Network.getResponseBody({ requestId: params.requestId })).body;
const { body, base64Encoded } = await this._client.Network.getResponseBody({ requestId: cdpResponse.requestId });
const encoding = base64Encoded ? 'base64' : 'utf8';

content = body;
rawContent = new Buffer(body, encoding);

if (rawContent.length.toString() === cdpResponse.response.headers['Content-Length']) {
// Response wasn't compressed so both buffers are the same
rawResponse = rawContent;
} else {
rawResponse = null; //TODO: Find a way to get this data
}
} catch (e) {
debug(`Body requested after connection closed for request ${params.requestId}`);
debug(`Body requested after connection closed for request ${cdpResponse.requestId}`);
/* HACK: This is to make https://github.com/MicrosoftEdge/Sonar/pull/144 pass.
There are some concurrency issues at the moment that are more visible in low powered machines and
when the CPU is highly used. The problem is most likely related to having pending requests but
Expand All @@ -255,21 +260,49 @@ class CDPCollector implements ICollector {
* Cancel all requests/remove all listeners when we do `close()`
*/
}
debug(`Content for ${resourceUrl} downloaded`);
debug(`Content for ${cdpResponse.response.url} downloaded`);

return { content, rawContent, rawResponse };
}

/** Returns a Response for the given request */
private async createResponse(cdpResponse): Promise<IResponse> {
const resourceUrl = cdpResponse.response.url;
const hops = this._redirects.calculate(resourceUrl);
const resourceHeaders = normalizeHeaders(cdpResponse.response.headers);

const {content, rawContent, rawResponse} = await this.getResponseBody(cdpResponse);

const response: IResponse = {
body: {
content: resourceBody,
content,
contentEncoding: getCharset(resourceHeaders),
rawContent: Buffer.alloc(params.response.encodedDataLength), //TODO: get the actual bytes!
rawResponse: null //TODO: get the real response bytes
rawContent,
rawResponse
},
headers: resourceHeaders,
hops,
statusCode: params.response.status,
url: params.response.url
statusCode: cdpResponse.response.status,
url: resourceUrl
};

return response;
}

/** Event handler fired when HTTP response is available and DOM loaded. */
private async onResponseReceived(params) {
// DOM is not ready so we queue up the event for later
if (!this._dom) {
this._pendingResponseReceived.push(this.onResponseReceived.bind(this, params));

return;
}
const resourceUrl = params.response.url;
const hops = this._redirects.calculate(resourceUrl);
const originalUrl = hops[0] || resourceUrl;

const response = await this.createResponse(params);

const request: IRequest = {
headers: params.response.requestHeaders,
url: originalUrl
Expand Down
1 change: 0 additions & 1 deletion src/lib/rules/content-type/content-type.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@ const rule: IRuleBuilder = {
description: 'Check usage of `Content-Type` HTTP response header'
},
fixable: 'code',
ignoredCollectors: ['cdp'], // TODO: Remove once #71 and #164 are fixed.
recommended: true,
schema: [{
items: { type: 'string' },
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/collectors/events.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const events = [
element: {
getAttribute(attr) {
if (attr === 'href') {
return 'style.css';
return '/script4.js';
}

return '';
Expand Down
Binary file added tests/lib/rules/content-type/fixtures/image.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
77 changes: 41 additions & 36 deletions tests/lib/rules/content-type/tests.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint no-undefined: 0 */

import * as fs from 'fs';

import { generateHTMLPage } from '../../../helpers/misc';
import { getRuleName } from '../../../../src/lib/utils/rule-helpers';
import { RuleTest } from '../../../helpers/rule-test-type'; // eslint-disable-line no-unused-vars
Expand All @@ -8,8 +10,7 @@ import * as ruleRunner from '../../../helpers/rule-runner';
const ruleName = getRuleName(__dirname);

// File content.

const pngFileContent = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]);
const pngFileContent = fs.readFileSync(`${__dirname}/fixtures/image.png`); // eslint-disable-line no-sync
const svgFileContent = '<svg xmlns="http://www.w3.org/2000/svg"><path d="M1,1"/></svg>';

// Error messages.
Expand Down Expand Up @@ -201,36 +202,40 @@ const testsForDefaults: Array<RuleTest> = [
'/test': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
}
},
{
name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=module')`,
reports: [{ message: generateIncorrectMediaTypeMessage('application/javascript', 'text/plain') }],
serverConfig: {
'/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="module" src="test"></script>')),
'/test': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
}
},
{
name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=text/plain' and 'js' file extension)`,
reports: [{ message: generateIncorrectMediaTypeMessage('application/javascript', 'text/plain') }],
serverConfig: {
'/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="text/plain" src="test.js"></script>')),
'/test.js': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
}
},
{
name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=text/plain' and 'tmpl' file extension)`,
serverConfig: {
'/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="text/plain" src="test.tmpl"></script>')),
'/test.tmpl': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
}
},
{
name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=simple/template' and 'tmpl' file extension)`,
serverConfig: {
'/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="text/plain" src="test.txt"></script>')),
'/test.txt': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
}
},
// TODO: Enable once Chrome has support for modules without a flag (https://www.chromestatus.com/feature/5365692190687232)
// {
// name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=module')`,
// reports: [{ message: generateIncorrectMediaTypeMessage('application/javascript', 'text/plain') }],
// serverConfig: {
// '/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="module" src="test"></script>')),
// '/test': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
// }
// },
// TODO: Chrome will not download if it doesn't like the type: https://github.com/MicrosoftEdge/Sonar/pull/245#discussion_r120083650, #250
// {
// name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=text/plain' and 'js' file extension)`,
// reports: [{ message: generateIncorrectMediaTypeMessage('application/javascript', 'text/plain') }],
// serverConfig: {
// '/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="text/plain" src="test.js"></script>')),
// '/test.js': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
// }
// },
// TODO: The following test passes in CDP because it doesn't download anything so no errors. Need to do #250 so we can keep testing elsewhere
// {
// name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=text/plain' and 'tmpl' file extension)`,
// serverConfig: {
// '/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="text/plain" src="test.tmpl"></script>')),
// '/test.tmpl': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
// }
// },
// TODO: The following test passes in CDP because it doesn't download anything so no errors. Need to do #250 so we can keep testing elsewhere
// {
// name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=simple/template' and 'tmpl' file extension)`,
// serverConfig: {
// '/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="text/plain" src="test.txt"></script>')),
// '/test.txt': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
// }
// },
{
name: `Stylesheet is served with 'Content-Type' header with the wrong media type`,
reports: [{ message: generateIncorrectMediaTypeMessage('text/css', 'font/woff') }],
Expand Down Expand Up @@ -282,23 +287,23 @@ const testsForDefaults: Array<RuleTest> = [

const testsForConfigs: Array<RuleTest> = [
{
name: `Script is served with the 'Content-Type' header with the correct media type but wrong because of the configs`,
name: `Script is served with the 'Content-Type' header with the correct media type but wrong because of the custom config`,
reports: [{ message: generateRequireValueMessage('text/javascript') }],
serverConfig: {
'/': generateHTMLPageData(generateHTMLPage(undefined, `<script src="test.js"></script>`)),
'/test.js': { headers: { 'Content-Type': 'application/javascript; charset=utf-8' } }
}
},
{
name: `Script is served with the 'Content-Type' header with the correct media type but wrong because of the overwritten configs`,
name: `Script is served with the 'Content-Type' header with the correct media type but fails because of the custom config overwrites`,
reports: [{ message: generateRequireValueMessage('application/x-javascript; charset=utf-8') }],
serverConfig: {
'/': generateHTMLPageData(generateHTMLPage(undefined, `<script src="test/test2.js"></script>`)),
'/test.js': { headers: { 'Content-Type': 'text/javascript' } }
'/test/test2.js': { headers: { 'Content-Type': 'text/javascript' } }
}
},
{
name: `Script is served with the 'Content-Type' header with the incorrect media type but correct because of the configs`,
name: `Script is served with the 'Content-Type' header with the incorrect media type but passes because of the custom config`,
serverConfig: {
'/': generateHTMLPageData(generateHTMLPage(undefined, `<script src="test3.js"></script>`)),
'/test3.js': { headers: { 'Content-Type': 'application/x-javascript' } }
Expand Down

0 comments on commit 61ca59e

Please sign in to comment.