Skip to content

Commit

Permalink
Chore: Migrate to using node's WHATWG URL API
Browse files Browse the repository at this point in the history
Fix #871
Close #903
  • Loading branch information
sarvaje authored and alrra committed Mar 22, 2018
1 parent b0879d2 commit 1000c13
Show file tree
Hide file tree
Showing 26 changed files with 105 additions and 107 deletions.
10 changes: 2 additions & 8 deletions packages/rule-apple-touch-icons/src/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* @fileoverview Check for correct usage of `apple-touch-icon`.
*/

import * as url from 'url';
import { URL } from 'url';

import * as getImageData from 'image-size';

Expand Down Expand Up @@ -99,18 +99,12 @@ export default class AppleTouchIconsRule implements IRule {

// - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

let appleTouchIconURL = '';

/*
* If `href` exists and is not an empty string, try
* to figure out the full URL of the `apple-touch-icon`.
*/

if (url.parse(appleTouchIconHref).protocol) {
appleTouchIconURL = appleTouchIconHref;
} else {
appleTouchIconURL = url.resolve(resource, appleTouchIconHref);
}
const appleTouchIconURL = new URL(appleTouchIconHref, resource).href;

// - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Expand Down
8 changes: 2 additions & 6 deletions packages/rule-disown-opener/src/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* ------------------------------------------------------------------------------
*/

import * as url from 'url';
import { URL } from 'url';

import { isSupported } from 'caniuse-api';
import * as pluralize from 'pluralize';
Expand Down Expand Up @@ -72,11 +72,7 @@ export default class DisownOpenerRule implements IRule {
const checkSameOrigin = (resource: string, element: IAsyncHTMLElement): boolean => {
const hrefValue: string = normalizeString(element.getAttribute('href'));

let fullURL: string = hrefValue;

if (!url.parse(hrefValue).protocol) {
fullURL = url.resolve(resource, hrefValue);
}
const fullURL: string = new URL(hrefValue, resource).href;

/*
* Same origin URLs are ignored by default, but users can
Expand Down
3 changes: 2 additions & 1 deletion packages/rule-no-friendly-error-pages/src/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

import * as url from 'url';
import { URL } from 'url'; // this is necessary to avoid TypeScript mixes types.

import { Category } from 'sonarwhal/dist/src/lib/enums/category';
import { debug as d } from 'sonarwhal/dist/src/lib/utils/debug';
Expand Down Expand Up @@ -91,7 +92,7 @@ export default class NoFriendlyErrorPagesRule implements IRule {
};

const tryToGenerateErrorPage = async (targetURL: string) => {
const baseURL: string = url.format(Object.assign(url.parse(targetURL), {
const baseURL: string = url.format(Object.assign(new URL(targetURL), {
fragment: false,
search: false
}));
Expand Down
4 changes: 2 additions & 2 deletions packages/rule-performance-budget/src/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* @fileoverview Performance budget checks if your site will load fast enough based on the size of your resources and a given connection speed
*/

import * as url from 'url';
import { URL } from 'url';

import { Category } from 'sonarwhal/dist/src/lib/enums/category';
import { debug as d } from 'sonarwhal/dist/src/lib/utils/debug';
Expand Down Expand Up @@ -74,7 +74,7 @@ export default class PerformanceBudgetRule implements IRule {

/** Update the stored information for unique domains (`uniqueDomains`) and connections over https (`secureDomains`). */
const updateDomainsInfo = (resource: string) => {
const resourceUrl = url.parse(resource);
const resourceUrl = new URL(resource);

uniqueDomains.add(resourceUrl.hostname);

Expand Down
5 changes: 3 additions & 2 deletions packages/rule-strict-transport-security/src/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* @fileoverview Check if responses served over HTTPS also have the Strict-Transport-Security header with a proper value max-age value.
*/
import * as url from 'url';
import { URL } from 'url'; // this is necessary to avoid TypeScript mixes types.

import { Category } from 'sonarwhal/dist/src/lib/enums/category';
import { RuleContext } from 'sonarwhal/dist/src/lib/rule-context';
Expand Down Expand Up @@ -111,7 +112,7 @@ export default class StrictTransportSecurityRule implements IRule {
};

const verifyPreload = async (resource): Promise<{ [key: string]: any }> => {
const originalDomain = url.parse(resource).hostname;
const originalDomain = new URL(resource).hostname;
const mainDomain = originalDomain.replace(/^www./, '');
// Some hostnames in the list include `www.`, e.g., `www.gov.uk`.
let status: string;
Expand Down Expand Up @@ -168,7 +169,7 @@ export default class StrictTransportSecurityRule implements IRule {
}

if (!isHTTPS(resource) && !headerValue) {
const httpsResource = url.format(Object.assign(url.parse(resource), { protocol: `https` }));
const httpsResource = url.format(Object.assign(new URL(resource), { protocol: `https` }));

try {
const networkData: NetworkData = await context.fetchContent(httpsResource);
Expand Down
3 changes: 2 additions & 1 deletion packages/sonarwhal/src/lib/cli/analyze.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { promisify } from 'util';
import { URL } from 'url';

import * as async from 'async';
import * as inquirer from 'inquirer';
Expand All @@ -7,7 +8,7 @@ import * as pluralize from 'pluralize';

import { SonarwhalConfig } from '../config';
import { Sonarwhal } from '../sonarwhal';
import { CLIOptions, ORA, Problem, Severity, URL } from '../types';
import { CLIOptions, ORA, Problem, Severity } from '../types';
import { debug as d } from '../utils/debug';
import { getAsUris } from '../utils/get-as-uri';
import * as logger from '../utils/logging';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* ------------------------------------------------------------------------------
*/

import * as url from 'url';
import { URL } from 'url';

import * as cdp from 'chrome-remote-interface';
import * as _ from 'lodash';
Expand All @@ -27,7 +27,7 @@ import { resolveUrl } from '../utils/resolver';
import {
BrowserInfo, IConnector,
IAsyncHTMLElement, ElementFound, Event, FetchEnd, FetchError, ILauncher, ManifestFetchError, TraverseUp, TraverseDown,
Response, Request, NetworkData, URL
Response, Request, NetworkData
} from '../../types';

import { normalizeHeaders } from '../utils/normalize-headers';
Expand Down Expand Up @@ -158,7 +158,7 @@ export class Connector implements IConnector {
// We need to calculate the original url because it might have redirects
const originalUrl: Array<string> = this._redirects.calculate(requestUrl);

requestUrl = url.parse(originalUrl[0] || requestUrl);
requestUrl = new URL(originalUrl[0] || requestUrl);
const parts: Array<string> = requestUrl.href.split('/');

/*
Expand All @@ -177,7 +177,7 @@ export class Connector implements IConnector {
if (!this._finalHref) {
return false;
}
const faviconUrl = url.resolve(this._finalHref, '/favicon.ico');
const faviconUrl = new URL('/favicon.ico', this._finalHref).href;
const event = params.request || params.response;

if (!event) {
Expand Down Expand Up @@ -521,7 +521,7 @@ export class Connector implements IConnector {
}

private async getManifestManually(element: IAsyncHTMLElement) {
const manifestURL = resolveUrl(element.getAttribute('href'), this._finalHref);
const manifestURL: string = resolveUrl(element.getAttribute('href'), this._finalHref);

/*
* Try to see if the web app manifest file actually
Expand Down Expand Up @@ -748,7 +748,7 @@ export class Connector implements IConnector {
debug(`resource ${href} to be fetched`);
await this._server.emitAsync('fetch::start', { resource: href });

const content = await this.fetchContent(url.parse(this._finalHref + href.substr(1)));
const content = await this.fetchContent(new URL(this._finalHref + href.substr(1)));

const data: FetchEnd = {
element: null,
Expand Down
12 changes: 7 additions & 5 deletions packages/sonarwhal/src/lib/connectors/jsdom/jsdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@

import * as path from 'path';
import * as url from 'url';
import { URL } from 'url'; // this is necessary to avoid TypeScript mixes types.

import * as util from 'util';
import { fork, ChildProcess } from 'child_process';

Expand All @@ -37,7 +39,7 @@ import { getContentTypeData, getType } from '../../utils/content-type';
import {
IConnector,
ElementFound, Event, FetchEnd, FetchError, ManifestFetchError, TraverseDown, TraverseUp,
NetworkData, URL
NetworkData
} from '../../types';
import { JSDOMAsyncHTMLElement, JSDOMAsyncHTMLDocument } from './jsdom-async-html';
import { resolveUrl } from '../utils/resolver';
Expand Down Expand Up @@ -166,8 +168,8 @@ export default class JSDOMConnector implements IConnector {
let resourceUrl: string = resource.url.href;
const element = resource.element ? new JSDOMAsyncHTMLElement(resource.element) : null;

if (!url.parse(resourceUrl).protocol) {
resourceUrl = url.resolve(this._finalHref, resourceUrl);
if (!resource.url.protocol) {
resourceUrl = new URL(resource.url.href, this._finalHref).href;
}

// Ignore if the resource has already been fetched.
Expand Down Expand Up @@ -231,7 +233,7 @@ export default class JSDOMConnector implements IConnector {
const href = (element && element.getAttribute('href')) || '/favicon.ico';

try {
await util.promisify(this.resourceLoader).call(this, { element, url: url.parse(href) });
await util.promisify(this.resourceLoader).call(this, { element, url: new URL(href, this._finalHref) });
} catch (e) {
debug('Error loading ${href}', e);
}
Expand Down Expand Up @@ -485,7 +487,7 @@ export default class JSDOMConnector implements IConnector {
* to get the right protocol but it doesn't seem return the right value.
*/
parsedTarget = parsedTarget.indexOf('//') === 0 ? `http:${parsedTarget}` : parsedTarget;
parsedTarget = url.parse(parsedTarget);
parsedTarget = new URL(parsedTarget);

return this.fetchContent(parsedTarget, customHeaders);
}
Expand Down
5 changes: 2 additions & 3 deletions packages/sonarwhal/src/lib/connectors/local/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ import * as logger from '../../utils/logging';

import {
IConnector,
Event, FetchEnd, ScanEnd,
URL
Event, FetchEnd, ScanEnd
} from '../../types';
// import { Requester } from '../utils/requester';
import { Sonarwhal } from '../../sonarwhal';
Expand Down Expand Up @@ -248,7 +247,7 @@ export default class LocalConnector implements IConnector {
* ------------------------------------------------------------------------------
*/

public async collect(target: URL) {
public async collect(target: url.URL) {
/** The target in string format */
const href: string = this._href = target.href;
const initialEvent: Event = { resource: href };
Expand Down
10 changes: 3 additions & 7 deletions packages/sonarwhal/src/lib/connectors/utils/resolver.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as url from 'url';
import { URL } from 'url';

/**
* Returns the full url domain if it is relative/absolute to a domain or
Expand All @@ -11,10 +11,6 @@ import * as url from 'url';
* -> `'http://example.com/favicon.ico'`
*
*/
export const resolveUrl = (href: string, baseUrl: string) => {
if (url.parse(href).protocol) {
return href;
}

return url.resolve(baseUrl, href);
export const resolveUrl = (href: string, baseUrl: string): string => {
return new URL(href, baseUrl).href;
};
8 changes: 4 additions & 4 deletions packages/sonarwhal/src/lib/sonarwhal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export class Sonarwhal extends EventEmitter {
this.on(eventName, createEventHandler(listener, id));
}

public fetchContent(target: string | url.Url, headers: object): Promise<NetworkData> {
public fetchContent(target: string | url.URL, headers: object): Promise<NetworkData> {
return this.connector.fetchContent(target, headers);
}

Expand All @@ -220,7 +220,7 @@ export class Sonarwhal extends EventEmitter {
this.messages.push(problem);
}

public clean(fileUrl: url.Url) {
public clean(fileUrl: url.URL) {
const file = url.format(fileUrl);

_.remove(this.messages, (message) => {
Expand All @@ -237,11 +237,11 @@ export class Sonarwhal extends EventEmitter {
}

/** Runs all the configured rules and plugins on a target */
public async executeOn(target: url.Url): Promise<Array<Problem>> {
public async executeOn(target: url.URL): Promise<Array<Problem>> {

const start: number = Date.now();

debug(`Starting the analysis on ${target.path}`);
debug(`Starting the analysis on ${target.href}`);

await this.connector.collect(target);

Expand Down
5 changes: 0 additions & 5 deletions packages/sonarwhal/src/lib/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import * as url from 'url';

import { IFormatterConstructor } from './types/formatters';
import { IConnectorConstructor } from './types/connector';
import { IParserConstructor } from './types/parser';
Expand Down Expand Up @@ -68,9 +66,6 @@ export type UserConfig = {
/** A resource required by sonarwhal: Connector, Formatter, Rule. */
export type Resource = IConnectorConstructor | IFormatterConstructor | IRuleConstructor;

/** An alias for url.Url. */
export type URL = url.Url;

export type CLIOptions = {
_: Array<string>;
config: string;
Expand Down
4 changes: 2 additions & 2 deletions packages/sonarwhal/src/lib/types/connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ export interface IConnector {
/** The headers from the response if applicable. */
headers?: object;
/** Collects all the information for the given target. */
collect(target: url.Url): Promise<any>;
collect(target: url.URL): Promise<any>;
/** Releases any used resource and/or browser. */
close(): Promise<void>;
/** Download an external resource using ` customHeaders` if needed. */
fetchContent?(target: url.Url | string, customHeaders?: object): Promise<NetworkData>;
fetchContent?(target: url.URL | string, customHeaders?: object): Promise<NetworkData>;
/** Evaluates the given JavaScript `code` asynchronously in the target. */
evaluate?(code: string): Promise<any>;
/** Finds all the nodes that match the given query. */
Expand Down
2 changes: 1 addition & 1 deletion packages/sonarwhal/src/lib/utils/get-as-path-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as url from 'url';
* * `file:///c:/projects/` --> `c:/projects/`
* * `file:///mnt/projects/` --> `/mnt/projects/`
*/
export const getAsPathString = (uri: url.Url) => {
export const getAsPathString = (uri: url.URL) => {

if (uri.protocol !== 'file:') {
return uri.pathname;
Expand Down
24 changes: 16 additions & 8 deletions packages/sonarwhal/src/lib/utils/get-as-uri.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as url from 'url';
import { URL } from 'url'; // this is necessary to avoid TypeScript mixes types.

import * as _ from 'lodash';
import * as fileUrl from 'file-url';
Expand All @@ -16,10 +17,17 @@ const debug: debug.IDebugger = d(__filename);
* * null if not valid
*
*/
export const getAsUri = (source: string): url.Url => {
export const getAsUri = (source: string): URL => {
const entry: string = source.trim();
let target: url.Url = url.parse(entry);
const protocol: string = target.protocol;
let target: URL;

try {
target = new URL(entry);
} catch (err) {
target = null;
}

const protocol: string = target ? target.protocol : null;

/*
* If it's a URI.
Expand All @@ -36,13 +44,13 @@ export const getAsUri = (source: string): url.Url => {
* If it does exist and it's a regular file.
*/
if (isFile(entry) || isDirectory(entry)) {
target = url.parse(fileUrl(entry));
target = new URL(fileUrl(entry));
debug(`Adding valid target: ${url.format(target)}`);

return target;
}

target = url.parse(`http://${entry}`);
target = new URL(`http://${entry}`);

/*
* And it doesn't exist locally, and is a valid URL:
Expand All @@ -69,9 +77,9 @@ export const getAsUri = (source: string): url.Url => {
* * null if not valid
*
*/
export const getAsUris = (source: Array<string>): Array<url.Url> => {
const targets: Array<url.Url> = source.reduce((uris: Array<url.Url>, entry: string): Array<url.Url> => {
const uri: url.Url = getAsUri(entry);
export const getAsUris = (source: Array<string>): Array<URL> => {
const targets: Array<URL> = source.reduce((uris: Array<URL>, entry: string): Array<URL> => {
const uri: URL = getAsUri(entry);

if (uri) {
uris.push(uri);
Expand Down
Loading

0 comments on commit 1000c13

Please sign in to comment.