Skip to content
Permalink
Browse files

Fix: Error location for HTTP headers and added code snippet

Fix #2209
Close #2269
  • Loading branch information...
sarvaje authored and antross committed Apr 18, 2019
1 parent 4ca99b1 commit b7b4866def7a5bad3cddcb05b49a51b0c5b52bec
@@ -154,7 +154,7 @@ export default class CodeframeFormatter implements IFormatter {
totalWarnings++;
}

partial = `${severity}: ${msg.message} (${msg.hintId}) at ${resourceString}${msg.sourceCode ? `:${location.line}:${location.column}` : ''}\n`;
partial = `${severity}: ${msg.message} (${msg.hintId}) at ${resourceString}${(location.line !== -1 && location.column !== -1) ? `:${location.line}:${location.column}` : ''}\n`;

if (msg.sourceCode) {
partial += codeFrame(msg.sourceCode, location);
@@ -28,7 +28,7 @@
<% } %>
<% if (problem.sourceCode) { %>
<div class="rule-result__code">
<code><%= utils.cutCodeString(problem.sourceCode); %></code>
<code class="<%= problem.codeLanguage %>"><%= utils.cutCodeString(problem.sourceCode); %></code>
</div>
<% } %>
</div>
@@ -47,4 +47,4 @@
<% } %>
<% }); %>
</div>
</details>
</details>
@@ -14,6 +14,7 @@ import { MediaType, parse } from 'content-type';
import { debug as d } from '@hint/utils/dist/src/debug';
import { normalizeString } from '@hint/utils/dist/src/misc/normalize-string';
import { isDataURI } from '@hint/utils/dist/src/network/is-data-uri';
import { capitalizeHeaderName } from '@hint/utils/dist/src/network/capitalize-header-name';
import { normalizeHeaderValue } from '@hint/utils/dist/src/network/normalize-header-value';
import { IHint, FetchEnd } from 'hint/dist/src/lib/types';
import { isTextMediaType } from '@hint/utils/dist/src/content-type';
@@ -52,7 +53,7 @@ export default class ContentTypeHint implements IHint {
return results && results[1];
};

const validate = ({ element, resource, response }: FetchEnd) => {
const validate = ({ resource, response }: FetchEnd) => {
if (response.statusCode !== 200) {
debug(`Check does not apply to status code !== 200`);

@@ -67,11 +68,13 @@ export default class ContentTypeHint implements IHint {
}

const contentTypeHeaderValue: string | null = normalizeHeaderValue(response.headers, 'content-type');
const codeSnippet = `${capitalizeHeaderName('content-type')}: ${contentTypeHeaderValue}`;
const codeLanguage = 'http';

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

if (contentTypeHeaderValue === null) {
context.report(resource, `Response should include 'content-type' header.`, { element });
context.report(resource, `Response should include 'content-type' header.`);

return;
}
@@ -85,7 +88,7 @@ export default class ContentTypeHint implements IHint {

if (userDefinedMediaType) {
if (normalizeString(userDefinedMediaType) !== contentTypeHeaderValue) {
context.report(resource, `'content-type' header value should be '${userDefinedMediaType}'.`, { element });
context.report(resource, `'content-type' header value should be '${userDefinedMediaType}'.`, { codeLanguage, codeSnippet });
}

return;
@@ -102,7 +105,7 @@ export default class ContentTypeHint implements IHint {

contentType = parse(contentTypeHeaderValue);
} catch (e) {
context.report(resource, `'content-type' header value should be valid (${e.message}).`, { element });
context.report(resource, `'content-type' header value should be valid (${e.message}).`, { codeLanguage, codeSnippet });

return;
}
@@ -131,21 +134,21 @@ export default class ContentTypeHint implements IHint {
// * media type

if (mediaType && (mediaType !== originalMediaType)) {
context.report(resource, `'content-type' header media type value should be '${mediaType}', not '${originalMediaType}'.`, { element });
context.report(resource, `'content-type' header media type value should be '${mediaType}', not '${originalMediaType}'.`, { codeLanguage, codeSnippet });
}

// * charset value

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

@@ -4,6 +4,7 @@

import { debug as d } from '@hint/utils/dist/src/debug';
import { isDataURI } from '@hint/utils/dist/src/network/is-data-uri';
import { capitalizeHeaderName } from '@hint/utils/dist/src/network/capitalize-header-name';
import { normalizeHeaderValue } from '@hint/utils/dist/src/network/normalize-header-value';
import { IHint, FetchEnd } from 'hint/dist/src/lib/types';
import { HintContext } from 'hint/dist/src/lib/hint-context';
@@ -248,7 +249,7 @@ export default class HttpCacheHint implements IHint {
const cacheControl: string | null = headers && headers['cache-control'] || null;

if (!cacheControl) {
context.report(resource, `No "cache-control" header or empty value found. It should have a value`, { element: fetchEnd.element });
context.report(resource, `No "cache-control" header or empty value found. It should have a value`);

return false;
}
@@ -260,21 +261,23 @@ export default class HttpCacheHint implements IHint {
* Validates if all the cache-control directives and values are correct.
*/
const hasInvalidDirectives = (directives: ParsedDirectives, fetchEnd: FetchEnd): boolean => {
const { invalidDirectives, invalidValues } = directives;
const { header, invalidDirectives, invalidValues } = directives;
const { resource } = fetchEnd;
const codeSnippet = `${capitalizeHeaderName('cache-control')}: ${header}`;
const codeLanguage = 'http';

if (invalidDirectives.size > 0) {
const message: string = `The ${invalidDirectives.size === 1 ? 'directive' : 'directives'} ${Array.from(invalidDirectives.keys()).join(', ')} ${invalidDirectives.size === 1 ? 'is' : 'are'} invalid`;

context.report(resource, message, { element: fetchEnd.element });
context.report(resource, message, { codeLanguage, codeSnippet });

return false;
}

if (invalidValues.size > 0) {
const message: string = `The following ${invalidValues.size === 1 ? 'directive has' : 'directives have'} an invalid value:\n${directivesToString(invalidValues)}`;

context.report(resource, message, { element: fetchEnd.element });
context.report(resource, message, { codeLanguage, codeSnippet });

return false;
}
@@ -286,14 +289,14 @@ export default class HttpCacheHint implements IHint {
* Validates if there is any non recommended directives.
*/
const hasNoneNonRecommendedDirectives = (directives: ParsedDirectives, fetchEnd: FetchEnd): boolean => {
const { usedDirectives } = directives;
const { header, usedDirectives } = directives;
const { resource } = fetchEnd;
const nonRecommendedDirective = nonRecommendedDirectives(usedDirectives);

if (nonRecommendedDirective) {
const message: string = `The directive "${nonRecommendedDirective}" is not recommended`;

context.report(resource, message, { element: fetchEnd.element });
context.report(resource, message, { codeLanguage: 'http', codeSnippet: `${capitalizeHeaderName('cache-control')}: ${header}` });

return false;
}
@@ -314,7 +317,7 @@ export default class HttpCacheHint implements IHint {
if (hasMaxAge) {
const message: string = `The following Cache-Control header is using a wrong combination of directives:\n${header}`;

context.report(fetchEnd.resource, message, { element: fetchEnd.element });
context.report(fetchEnd.resource, message, { codeLanguage: 'http', codeSnippet: `${capitalizeHeaderName('cache-control')}: ${header}` });

return false;
}
@@ -338,7 +341,7 @@ export default class HttpCacheHint implements IHint {
if (!isValidCache) {
const message: string = `The target should not be cached, or have a small "max-age" value (${maxAgeTarget}):\n${header}`;

context.report(fetchEnd.resource, message, { element: fetchEnd.element });
context.report(fetchEnd.resource, message, { codeLanguage: 'http', codeSnippet: `${capitalizeHeaderName('cache-control')}: ${header}` });

return false;
}
@@ -351,7 +354,9 @@ export default class HttpCacheHint implements IHint {
*/
const hasLongCache = (directives: ParsedDirectives, fetchEnd: FetchEnd): boolean => {
const { header, usedDirectives } = directives;
const { resource, element } = fetchEnd;
const { resource } = fetchEnd;
const codeSnippet = `${capitalizeHeaderName('cache-control')}: ${header}`;
const codeLanguage = 'http';

const longCache = compareToMaxAge(usedDirectives, maxAgeResource) >= 0;
const immutable = usedDirectives.has('immutable');
@@ -361,15 +366,15 @@ export default class HttpCacheHint implements IHint {
if (usedDirectives.has('no-cache') || !longCache) {
const message: string = `Static resources should have a long cache value (${maxAgeResource}):\nDirectives used: ${header}`;

context.report(resource, message, { element });
context.report(resource, message, { codeLanguage, codeSnippet });

validates = false;
}

if (!immutable) {
const message: string = `Static resources should use the "immutable" directive:\nDirectives used: ${header}`;

context.report(resource, message, { element });
context.report(resource, message, { codeLanguage, codeSnippet });

validates = false;
}
@@ -19,7 +19,7 @@ import { CompressionCheckOptions } from './types';
import meta from './meta';

const { getFileExtension, isTextMediaType } = contentType;
const { isHTTP, isRegularProtocol, normalizeHeaderValue } = network;
const { capitalizeHeaderName, isHTTP, isRegularProtocol, normalizeHeaderValue } = network;
const { normalizeString } = misc;
const decompressBrotli = promisify(brotli.decompress) as (buffer: Buffer) => Promise<Buffer>;
const uaString = 'Mozilla/5.0 Gecko';
@@ -63,13 +63,24 @@ export default class HttpCompressionHint implements IHint {
return (normalizeHeaderValue(headers, headerName) || '').split(',');
};

const checkVaryHeader = (resource: string, element: HTMLElement | null, headers: HttpHeaders) => {
const checkVaryHeader = (resource: string, headers: HttpHeaders) => {
const varyHeaderValues = getHeaderValues(headers, 'vary');
const cacheControlValues = getHeaderValues(headers, 'cache-control');

if (!cacheControlValues.includes('private') &&
!varyHeaderValues.includes('accept-encoding')) {
context.report(resource, `Response should include 'vary' header containing 'accept-encoding' value.`, { element });

let codeSnippet = '';

if (varyHeaderValues.length > 0) {
codeSnippet = `${capitalizeHeaderName('vary')}: ${varyHeaderValues.join(',')}\n`;
}

if (cacheControlValues.length > 0) {
codeSnippet += `${capitalizeHeaderName('cache-control')}: ${cacheControlValues.join(',')}`;
}

context.report(resource, `Response should include 'vary' header containing 'accept-encoding' value.`, { codeLanguage: 'http', codeSnippet: codeSnippet.trim() });
}
};

@@ -305,7 +316,7 @@ export default class HttpCompressionHint implements IHint {

// Check related headers.

checkVaryHeader(resource, element, response.headers);
checkVaryHeader(resource, response.headers);

if (contentEncodingHeaderValue !== 'br') {
context.report(resource, generateContentEncodingMessage('br'), { element });
@@ -379,7 +390,7 @@ export default class HttpCompressionHint implements IHint {

if (shouldCheckIfCompressedWith.gzip ||
shouldCheckIfCompressedWith.zopfli) {
checkVaryHeader(resource, element, response.headers);
checkVaryHeader(resource, response.headers);

if (contentEncodingHeaderValue !== 'gzip') {
context.report(resource, generateContentEncodingMessage('gzip'), { element });
@@ -611,8 +622,10 @@ export default class HttpCompressionHint implements IHint {
if ((response.mediaType === 'image/svg+xml' || getFileExtension(resource) === 'svgz') &&
isCompressedWithGzip(rawResponse)) {

if (normalizeHeaderValue(response.headers, 'content-encoding') !== 'gzip') {
context.report(resource, generateContentEncodingMessage('gzip'), { element });
const headerValue = normalizeHeaderValue(response.headers, 'content-encoding');

if (headerValue !== 'gzip') {
context.report(resource, generateContentEncodingMessage('gzip'), { codeLanguage: 'http', codeSnippet: `${capitalizeHeaderName('content-encoding')}: ${headerValue}` });
}

return true;
@@ -672,7 +685,7 @@ export default class HttpCompressionHint implements IHint {

// * Check if resource is sent with the `Content-Encoding` header.
if (contentEncodingHeaderValue) {
context.report(resource, `Response should not include 'content-encoding' header.`, { element });
context.report(resource, `Response should not include 'content-encoding' header.`, { codeLanguage: 'http', codeSnippet: `${capitalizeHeaderName('content-encoding')}: ${contentEncodingHeaderValue}` });
}

return;
@@ -14,6 +14,7 @@ import { prettyPrintArray } from '@hint/utils/dist/src/misc/pretty-print-array';
import { toLowerCaseArray } from '@hint/utils/dist/src/misc/to-lowercase-array';
import { includedHeaders } from '@hint/utils/dist/src/network/included-headers';
import { isDataURI } from '@hint/utils/dist/src/network/is-data-uri';
import { capitalizeHeaderName } from '@hint/utils/dist/src/network/capitalize-header-name';
import { normalizeHeaderValue } from '@hint/utils/dist/src/network/normalize-header-value';

import { HintContext } from 'hint/dist/src/lib/hint-context';
@@ -102,7 +103,7 @@ export default class NoDisallowedHeadersHint implements IHint {
});
};

const validate = ({ element, response, resource }: FetchEnd) => {
const validate = ({ response, resource }: FetchEnd) => {
// This check does not make sense for data URI.

if (isDataURI(resource)) {
@@ -132,6 +133,7 @@ export default class NoDisallowedHeadersHint implements IHint {
*/

const serverHeaderValue = normalizeHeaderValue(response.headers, 'server');
const codeLanguage = 'http';

if (!disallowedHeaders.includes('server') &&
!toLowerCaseArray(ignoreHeaders).includes('server') &&
@@ -140,13 +142,17 @@ export default class NoDisallowedHeadersHint implements IHint {
) {
const message = `'server' header value should only contain the server name, not '${(response.headers as any).server}'.`;

context.report(resource, message, { element });
context.report(resource, message, { codeLanguage, codeSnippet: `${capitalizeHeaderName('server')}: ${serverHeaderValue}` });
}

if (numberOfHeaders > 0) {
const message = `Response should not include disallowed ${prettyPrintArray(headers)} ${numberOfHeaders === 1 ? 'header' : 'headers'}.`;

context.report(resource, message, { element });
const codeSnippet = headers.reduce((total, header) => {
return `${total}${total ? '\n' : ''}${header}: ${normalizeHeaderValue(response.headers, header)}`;
}, '');

context.report(resource, message, { codeLanguage, codeSnippet });
}
};

@@ -282,9 +282,10 @@ export class Engine<E extends Events = Events> extends EventEmitter {
}

/** Reports a message from one of the hints. */
public report(hintId: string, category: Category, severity: Severity, sourceCode: string, location: ProblemLocation | null, message: string, resource: string) {
public report(hintId: string, category: Category, severity: Severity, sourceCode: string, location: ProblemLocation | null, message: string, resource: string, codeLanguage: string | undefined) {
const problem: Problem = {
category,
codeLanguage,
hintId,
location: location || { column: -1, line: -1 },
message,
@@ -33,6 +33,8 @@ export type ReportOptions = {
location?: ProblemLocation | null;
/** The `Severity` to report the issue as (overrides default settings for a hint). */
severity?: Severity;
/** Indicate the language of the codeSnippet */
codeLanguage?: string;
};

/** Acts as an abstraction layer between hints and the main hint object. */
@@ -117,8 +119,8 @@ export class HintContext<E extends Events = Events> {
/** Reports a problem with the resource. */
public report(resource: string, message: string, options: ReportOptions = {}) {
const { codeSnippet, element, severity } = options;
let position = options.location || null;
let sourceCode: string | null = null;
let position = options.location || null;

if (element) {
// When element is provided, position is an offset in the content.
@@ -138,7 +140,8 @@ export class HintContext<E extends Events = Events> {
codeSnippet || sourceCode || '',
position,
message,
resource
resource,
options.codeLanguage
);
}

@@ -26,4 +26,6 @@ export type Problem = {
category: Category;
/** The severity of the hint based on the actual configuration */
severity: Severity;
/** Indicate the language of the sourceCode */
codeLanguage?: string;
};
@@ -0,0 +1,15 @@
import { toPascalCase } from '../misc/to-pascal-case';

/**
* Capitalize a header name.
*
* e.g:
* content-type => Content-Type
*/
export const capitalizeHeaderName = (headerName: string) => {
const parts = headerName.split('-').map((partialName) => {
return toPascalCase(partialName);
});

return parts.join('-');
};
Oops, something went wrong.

0 comments on commit b7b4866

Please sign in to comment.
You can’t perform that action at this time.