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.

Sign up for GitHub

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

Jump to bottom

Refactor and improve rules #256

Merged
merged 2 commits into from Jun 13, 2017
Merged

Refactor and improve rules #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4974344
Refactor and improve rules and their tests
alrra Jun 11, 2017
e63767a
Make rules that check headers ignore data URIs
alrra Jun 12, 2017
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
@@ -1,3 +1,4 @@
*.lock
*.log
*.pid
.DS_Store
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Expand Up @@ -38,6 +38,7 @@
"optionator": "^0.8.2",
"path-is-inside": "^1.0.2",
"pify": "^3.0.0",
"pluralize": "^5.0.0",
"request": "^2.79.0",
"require-uncached": "^1.0.3",
"same-origin": "^0.1.1",
Expand Down
33 changes: 23 additions & 10 deletions src/lib/rules/content-type/content-type.ts
View file
Open in desktop
Expand Up @@ -8,16 +8,20 @@
// ------------------------------------------------------------------------------

import * as path from 'path';
import * as url from 'url';

import { debug as d } from '../../utils/debug';
import * as fileType from 'file-type';
import * as isSvg from 'is-svg';
import * as mimeDB from 'mime-db';
import { parse } from 'content-type';

import { IAsyncHTMLElement, IResponseBody, IRule, IRuleBuilder, IFetchEndEvent } from '../../types'; // eslint-disable-line no-unused-vars
import { normalizeString } from '../../utils/misc';
import { isDataURI, normalizeString } from '../../utils/misc';
import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars

const debug = d(__filename);

// ------------------------------------------------------------------------------
// Public
// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -146,8 +150,8 @@ const rule: IRuleBuilder = {
};

const determineMediaTypeBasedOnFileExtension = (resource: string) => {
const fileExtension = path.extname(resource).split('.')
.pop();
const fileExtension = path.extname(url.parse(resource).pathname).split('.')
.pop();

return getMediaTypeBasedOnFileExtension(fileExtension);
};
Expand All @@ -170,12 +174,21 @@ const rule: IRuleBuilder = {

const validate = async (fetchEnd: IFetchEndEvent) => {
const { element, resource, response } = fetchEnd;

// This check does not make sense for data URIs.

if (isDataURI(resource)) {
debug(`Check does not apply for data URIs`);

return;
}

const contentTypeHeaderValue = normalizeString(response.headers['content-type']);

// Check if the `Content-Type` header was sent.

if (contentTypeHeaderValue === null) {
await context.report(resource, element, `'Content-Type' header was not specified`);
await context.report(resource, element, `'content-type' header was not specified`);

return;
}
Expand All @@ -187,7 +200,7 @@ const rule: IRuleBuilder = {

if (userDefinedMediaType) {
if (normalizeString(userDefinedMediaType) !== contentTypeHeaderValue) {
await context.report(resource, element, `'Content-Type' header should have the value: '${userDefinedMediaType}'`);
await context.report(resource, element, `'content-type' header should have the value '${userDefinedMediaType}'`);
}

return;
Expand All @@ -204,7 +217,7 @@ const rule: IRuleBuilder = {

contentType = parse(contentTypeHeaderValue);
} catch (e) {
await context.report(resource, element, `'Content-Type' header value is invalid (${e.message})`);
await context.report(resource, element, `'content-type' header value is invalid (${e.message})`);

return;
}
Expand All @@ -227,17 +240,17 @@ const rule: IRuleBuilder = {
// * media type

if (mediaType && (mediaType !== originalMediaType)) {
await context.report(resource, element, `'Content-Type' header should have media type: '${mediaType}' (not '${originalMediaType}')`);
await context.report(resource, element, `'content-type' header should have media type '${mediaType}' (not '${originalMediaType}')`);
}

// * charset value

if (charset) {
if (!originalCharset || (charset !== originalCharset)) {
await context.report(resource, element, `'Content-Type' header should have 'charset=${charset}'${originalCharset ? ` (not '${originalCharset}')` : ''}`);
await context.report(resource, element, `'content-type' header should have 'charset=${charset}'${originalCharset ? ` (not '${originalCharset}')` : ''}`);
}
} else if (originalCharset && !['text/html', 'application/xhtml+xml'].includes(originalMediaType)) {
await context.report(resource, element, `'Content-Type' header should not have 'charset=${originalCharset}'`);
await context.report(resource, element, `'content-type' header should not have 'charset=${originalCharset}'`);
}

};
Expand All @@ -253,7 +266,7 @@ const rule: IRuleBuilder = {
meta: {
docs: {
category: 'interoperability',
description: 'Check usage of `Content-Type` HTTP response header'
description: 'Require `Content-Type` header with appropriate value'
},
fixable: 'code',
recommended: true,
Expand Down
23 changes: 20 additions & 3 deletions src/lib/rules/disallowed-headers/disallowed-headers.ts
View file
Open in desktop
Expand Up @@ -6,10 +6,16 @@
// Requirements
// ------------------------------------------------------------------------------

import * as pluralize from 'pluralize';

import { debug as d } from '../../utils/debug';
import { getIncludedHeaders, mergeIgnoreIncludeArrays } from '../../utils/rule-helpers';
import { IFetchEndEvent, IRule, IRuleBuilder } from '../../types'; // eslint-disable-line no-unused-vars
import { isDataURI } from '../../utils/misc';
import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars

const debug = d(__filename);

// ------------------------------------------------------------------------------
// Public
// ------------------------------------------------------------------------------
Expand All @@ -35,24 +41,35 @@ const rule: IRuleBuilder = {

const validate = async (fetchEnd: IFetchEndEvent) => {
const { element, resource } = fetchEnd;

// This check does not make sense for data URI.

if (isDataURI(resource)) {
debug(`Check does not apply for data URI: ${resource}`);

return;
}

const headers = getIncludedHeaders(fetchEnd.response.headers, disallowedHeaders);
const numberOfHeaders = headers.length;

if (headers.length > 0) {
await context.report(resource, element, `Disallowed HTTP header${headers.length > 1 ? 's' : ''} found: ${headers.join(', ')}`);
if (numberOfHeaders > 0) {
await context.report(resource, element, `'${headers.join('\', \'')}' ${pluralize('header', numberOfHeaders)} ${pluralize('is', numberOfHeaders)} disallowed`);
}
};

loadRuleConfigs();

return {
'fetch::end': validate,
'manifestfetch::end': validate,
'targetfetch::end': validate
};
},
meta: {
docs: {
category: 'security',
description: 'Disallow certain HTTP headers'
description: 'Disallow certain HTTP response headers'
},
fixable: 'code',
recommended: true,
Expand Down
6 changes: 4 additions & 2 deletions src/lib/rules/disown-opener/disown-opener.ts
View file
Open in desktop
Expand Up @@ -10,8 +10,10 @@

import * as url from 'url';

import * as pluralize from 'pluralize';
import * as sameOrigin from 'same-origin';

import { cutString } from '../../utils/misc';
import { debug as d } from '../../utils/debug';
import { IElementFoundEvent, IRule, IRuleBuilder } from '../../types'; // eslint-disable-line no-unused-vars
import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars
Expand Down Expand Up @@ -78,7 +80,7 @@ const rule: IRuleBuilder = {
});

if (missingRelValues.length > 0) {
await context.report(resource, element, `Missing link type${missingRelValues.length === 1 ? '' : 's'} on \`${await element.outerHTML()}\`: ${missingRelValues.join(', ')}`, hrefValue);
await context.report(resource, element, `'${cutString(await element.outerHTML(), 100)}' is missing link ${pluralize('type', missingRelValues.length)} '${missingRelValues.join('\', \'')}'`, hrefValue);
}
};

Expand All @@ -103,7 +105,7 @@ const rule: IRuleBuilder = {
meta: {
docs: {
category: 'security',
description: 'Use `noopener` and `noreferrer` on `a` and `area` element with target="_blank"'
description: 'Require `noopener` (and `noreferrer`) on `a` and `area` element with target="_blank"'
},
fixable: 'code',
recommended: true,
Expand Down
31 changes: 15 additions & 16 deletions src/lib/rules/highest-available-document-mode/highest-available-document-mode.ts
View file
Open in desktop
Expand Up @@ -7,9 +7,8 @@
// Requirements
// ------------------------------------------------------------------------------

import * as url from 'url';

import { IAsyncHTMLDocument, IElementFoundEvent, IRule, IRuleBuilder, ITraverseEndEvent } from '../../types'; // eslint-disable-line no-unused-vars
import { IAsyncHTMLDocument, IRule, IRuleBuilder, ITraverseEndEvent } from '../../types'; // eslint-disable-line no-unused-vars
import { isLocalFile, normalizeString } from '../../utils/misc';
import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars

// ------------------------------------------------------------------------------
Expand All @@ -30,14 +29,14 @@ const rule: IRuleBuilder = {
const getXUACompatibleMetaTags = (elements) => {
return elements.filter((element) => {
return (element.getAttribute('http-equiv') !== null &&
element.getAttribute('http-equiv').toLowerCase() === 'x-ua-compatible');
normalizeString(element.getAttribute('http-equiv')) === 'x-ua-compatible');
});
};

const checkHeader = async (resource: string, responseHeaders: object) => {
const headerValue = responseHeaders['x-ua-compatible'];
const headerValue = normalizeString(responseHeaders['x-ua-compatible']);

if (typeof headerValue === 'undefined') {
if (headerValue === null) {

// There is no need to require the HTTP header if:
//
Expand All @@ -46,7 +45,7 @@ const rule: IRuleBuilder = {
// support document modes

if (!requireMetaTag && !suggestRemoval) {
await context.report(resource, null, `Response does not include the 'X-UA-Compatible' header`);
await context.report(resource, null, `'x-ua-compatible' header was not specified`);
}

return;
Expand All @@ -57,13 +56,13 @@ const rule: IRuleBuilder = {
// modes, suggest not sending the header.

if (suggestRemoval) {
await context.report(resource, null, `'X-UA-Compatible' HTTP response header is not needed`);
await context.report(resource, null, `'x-ua-compatible' header is not needed`);

return;
}

if (headerValue.toLowerCase() !== 'ie=edge') {
await context.report(resource, null, `The value of the 'X-UA-Compatible' HTTP response header should be 'ie=edge'`);
if (headerValue !== 'ie=edge') {
await context.report(resource, null, `'x-ua-compatible' header value should be 'ie=edge'`);
}

// Note: The check if the X-UA-Compatible HTTP response
Expand Down Expand Up @@ -94,7 +93,7 @@ const rule: IRuleBuilder = {
// If the user requested the meta tag to be specified.

if (XUACompatibleMetaTags.length === 0) {
await context.report(resource, null, `No 'X-UA-Compatible' meta tag was specified`);
await context.report(resource, null, `No 'x-ua-compatible' meta tag was specified`);

return;
}
Expand All @@ -103,7 +102,7 @@ const rule: IRuleBuilder = {
// the user intended to use, and check if:

const XUACompatibleMetaTag = XUACompatibleMetaTags[0];
const contentValue = (XUACompatibleMetaTag.getAttribute('content') || '').toLowerCase();
const contentValue = normalizeString(XUACompatibleMetaTag.getAttribute('content'));

// * it has the value `ie=edge`.

Expand Down Expand Up @@ -149,7 +148,7 @@ const rule: IRuleBuilder = {
const metaTags = XUACompatibleMetaTags.slice(1);

for (const metaTag of metaTags) {
await context.report(resource, metaTag, `A 'X-UA-Compatible' meta tag was already specified`);
await context.report(resource, metaTag, `A 'x-ua-compatible' meta tag was already specified`);
}
}
};
Expand All @@ -168,9 +167,9 @@ const rule: IRuleBuilder = {
const validate = async (event: ITraverseEndEvent) => {
const { resource } = event;

// The following check don't make sense for local files.
// The following check don't make for local files.

if (url.parse(resource).protocol !== 'file:') {
if (!isLocalFile(resource)) {
checkHeader(resource, context.pageHeaders);
}

Expand All @@ -184,7 +183,7 @@ const rule: IRuleBuilder = {
meta: {
docs: {
category: 'interoperability',
description: 'Use highest available document mode'
description: 'Require highest available document mode'
},
fixable: 'code',
recommended: true,
Expand Down
5 changes: 3 additions & 2 deletions src/lib/rules/manifest-exists/manifest-exists.ts
View file
Open in desktop
Expand Up @@ -9,6 +9,7 @@

import { debug as d } from '../../utils/debug';
import { IElementFoundEvent, IManifestFetchEnd, IManifestFetchErrorEvent, ITraverseEndEvent, IRule, IRuleBuilder } from '../../types'; // eslint-disable-line no-unused-vars
import { normalizeString } from '../../utils/misc';
import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars

const debug = d(__filename);
Expand All @@ -31,7 +32,7 @@ const rule: IRuleBuilder = {
const manifestExists = async (data: IElementFoundEvent) => {
const { element, resource } = data;

if (element.getAttribute('rel') !== 'manifest') {
if (normalizeString(element.getAttribute('rel')) !== 'manifest') {

return;
}
Expand Down Expand Up @@ -78,7 +79,7 @@ const rule: IRuleBuilder = {
meta: {
docs: {
category: 'pwa',
description: 'Provide a web app manifest file'
description: 'Require a web app manifest'
},
fixable: 'code',
recommended: true,
Expand Down
9 changes: 5 additions & 4 deletions src/lib/rules/manifest-file-extension/manifest-file-extension.ts
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@ import * as path from 'path';

import { debug as d} from '../../utils/debug';
import { IElementFoundEvent, IRule, IRuleBuilder } from '../../types'; // eslint-disable-line no-unused-vars
import { normalizeString } from '../../utils/misc';
import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars

const debug = d(__filename);
Expand All @@ -27,14 +28,14 @@ const rule: IRuleBuilder = {
const validate = async (data: IElementFoundEvent) => {
const { element, resource } = data;

if (element.getAttribute('rel') === 'manifest') {
const href = element.getAttribute('href');
if (normalizeString(element.getAttribute('rel')) === 'manifest') {
const href = normalizeString(element.getAttribute('href'));
const fileExtension = path.extname(href);

if (fileExtension !== standardManifestFileExtension) {
debug('Web app manifest file with invalid extension found');

await context.report(resource, element, `The file extension for the web app manifest file ('${href}') should be '${standardManifestFileExtension}' not '${fileExtension}'`, fileExtension);
await context.report(resource, element, `The file extension should be '${standardManifestFileExtension}' (not '${fileExtension}')`, fileExtension);
}
}
};
Expand All @@ -44,7 +45,7 @@ const rule: IRuleBuilder = {
meta: {
docs: {
category: 'pwa',
description: 'Use `.webmanifest` as the file extension for the web app manifest file'
description: 'Require `.webmanifest` as the file extension for the web app manifest file'
},
fixable: 'code',
recommended: true,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/rules/manifest-is-valid/manifest-is-valid.ts
View file
Open in desktop
Expand Up @@ -48,7 +48,7 @@ const rule: IRuleBuilder = {
meta: {
docs: {
category: 'pwa',
description: 'Check if the content of the web app manifest is valid'
description: 'Require valid web app manifest'
},
fixable: 'code',
recommended: true,
Expand Down