Skip to content
Permalink
Browse files

Breaking: Validate `Connector`s configuration

* Add a static property `schema` to the constructors and validate it
  during runtime at the same time as the `hint` ones are checked.
* Common property `ignoreHTTPSErrors` to ignore certificate issues.
* Remove some properties that were only necessary for
  `connector-edge`.
* Update documentation.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Fix #2257
Fix #2332
  • Loading branch information...
molant committed May 7, 2019
1 parent 2e15dbe commit fa1652bb1d99ffc1163dcca337836d9048832ac9
@@ -82,4 +82,4 @@ The set of settings supported by Chrome connector are:
[cli flags]: https://github.com/GoogleChrome/chrome-launcher/blob/master/docs/chrome-flags-for-tools.md
[connectors]: https://webhint.io/docs/user-guide/concepts/connectors/
[hintrc]: https://webhint.io/docs/user-guide/configuring-webhint/summary/
[chrome-launcher-issue]: https://github.com/GoogleChrome/chrome-launcher/issues/118
[chrome-launcher-issue]: https://github.com/GoogleChrome/chrome-launcher/issues/118
@@ -17,8 +17,20 @@ import { CDPLauncher } from './chrome-launcher';


export default class ChromeConnector extends Connector {
public constructor(server: Engine, config?: object) {
const launcher: ILauncher = new CDPLauncher(config || {});
public static schema = {
additionalProperties: false,
properties: {
ignoreHTTPSErrors: { type: 'boolean' },
launcherOptions: { type: 'object' },
waitFor: {
minimum: 0,
type: 'number'
}
}
};

public constructor(server: Engine, config?: any) {
const launcher: ILauncher = new CDPLauncher(config && config.launcherOptions|| {});

super(server, config || {}, launcher);
}
@@ -49,7 +49,10 @@ const { getContentTypeData, getType } = contentType;
const { isHTMLDocument } = network;
const debug: debug.IDebugger = d(__filename);

const defaultOptions = { waitFor: 1000 };
const defaultOptions = {
ignoreHTTPSErrors: false,
waitFor: 5000
};

export default class JSDOMConnector implements IConnector {
private _options: any;
@@ -62,14 +65,36 @@ export default class JSDOMConnector implements IConnector {
private _resourceLoader?: ResourceLoader;
private _subprocesses: Set<ChildProcess>;

public static schema = {
additionalProperties: false,
properties: {
ignoreHTTPSErrors: { type: 'boolean' },
requestOptions: { type: 'object' },
waitFor: {
minimum: 0,
type: 'number'
}
}
};

public request: Requester;
public server: Engine;
public finalHref!: string;
public fetchedHrefs!: Set<string>;

public constructor(server: Engine, config?: object) {
public constructor(server: Engine, config?: any) {
this._options = Object.assign({}, defaultOptions, config);
this.request = new Requester(this._options);

const requesterOptions = Object.assign(
{},
{
rejectUnauthorized: !this._options.ignoreHTTPSErrors,
strictSSL: !this._options.ignoreHTTPSErrors
},
this._options.requestOptions || {}
);

this.request = new Requester(requesterOptions);
this.server = server;
this._timeout = server.timeout;
this._subprocesses = new Set();
@@ -102,8 +127,8 @@ export default class JSDOMConnector implements IConnector {

const r: Requester = new Requester({
headers: customHeaders,
rejectUnauthorized: this._options.rejectUnauthorized,
strictSSL: this._options.strictSSL
rejectUnauthorized: !this._options.ignoreHTTPSErrors,
strictSSL: !this._options.ignoreHTTPSErrors
});

return r.get(uri);
@@ -24,6 +24,16 @@ export default class WebExtensionConnector implements IConnector {
private _onComplete: (resource: string) => void = () => { };
private _options: ConnectorOptionsConfig;

public static schema = {
additionalProperties: false,
properties: {
waitFor: {
minimum: 0,
type: 'number'
}
}
};

public constructor(engine: Engine, options?: ConnectorOptionsConfig) {
this._engine = engine;
this._options = options || {};
@@ -55,6 +55,9 @@ with the values you want to modify:

The following is the list of shared configurations for all `connector`s:

* `ignoreHTTPSErrors` is a boolean that indicates if errors with certificates
should be ignored. Use this when checking self-signed certificated.

* `waitFor` time in milliseconds the connector will wait after the site is
ready before starting the DOM traversing and stop listening to any
network request. By default, it will wait until the network is somehow
@@ -64,53 +67,41 @@ Depending on the `connector`, other configurations may be available.

### jsdom configuration

`jsdom` allows you to configure the following:

* `headers`: the headers used to fetch the resources. By default they are:
`jsdom` is built on top of [`request`][request]. The way to configure
it is via the `requestOptions` property. The following is an example
on how to use custom headers:

```json
{
"Accept-Language": "en-US,en;q=0.8,es;q=0.6,fr;q=0.4",
"Cache-Control": "no-cache",
"DNT": 1,
"Pragma": "no-cache",
"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36"
"name": "jsdom",
"waitFor": 10000,
"requestOptions": {
"headers": {
"Accept-Language": "en-US,en;q=0.8,es;q=0.6,fr;q=0.4",
"Cache-Control": "no-cache",
"DNT": 1,
"Pragma": "no-cache",
"User-Agent": "Custom user agent"
}
}
}
```

### remote-debugging-connector configuration

There are some `connector`s built on top of the [Chrome DevTools
Protocol][cdp]. `chrome` and `edge` are some of these `connector`s.

The set of settings specific for them are:

* `defaultProfile (boolean)`: Indicates if the browser should use the
default profile or create a new one. By default the value is `false`
so a new one is created. You might want to set it to `true` if you
want `webhint` to have access to pages where the default profile is
already authenticated. This only applies for Google Chrome as
Microsoft Edge doesn’t create a new profile.
* `useTabUrl (boolean)`: Indicates if the browser should navigate first
to a given page before going to the final target. `false` by default.
* `tabUrl (string)`: The URL to visit before the final target in case
`useTabUrl` is `true`. `https://empty.webhint.io/` is the
default value.
* `flags? (string[])`: Allows you to pass in additional Chrome
command line API flags. Useful if you would like to start your
session in headless mode or with GPU disabled. Here's the full list
of [available command line flags][cli flags].
* `waitForContentLoaded (number)`: Time in milliseconds to wait for the
`loadingFinished` event from the `debugging protocol` before requesting
the body of a response. The default value is `10000` (10 seconds).
### chrome configuration

The `chrome-connector` uses [`chrome-launcher`][chrome-launcher] to
start the browser. The way to pass custom options is via the
`launcherOptions` property. The following is an example that passes
some `flags` to it and uses the `defaultProfile`:

```json
{
"defaultProfile": true,
"flags": ["--headless", "--disable-gpu"],
"tabUrl": "https://empty.webhint.io/",
"useTabUrl": false,
"waitForContentLoaded": 10000
{
"name": "chrome",
"waitFor": 10000,
"launcherOptions": {
"defaultProfile": true,
"flags": ["--headless", "--disable-gpu"],
}
}
```

@@ -168,9 +159,8 @@ connectors.

<!-- Link labels: -->

[cdp]: https://chromedevtools.github.io/devtools-protocol/
[cli flags]: https://github.com/GoogleChrome/chrome-launcher/blob/master/docs/chrome-flags-for-tools.md
[eda]: https://github.com/Microsoft/edge-diagnostics-adapter
[how to connector]: ../../contributor-guide/how-to/connector.md
[jsdom]: https://github.com/tmpvar/jsdom
[request]: https://github.com/request/request
[chrome-launcher]: https://github.com/googlechrome/chrome-launcher
[wsl-interop]: https://msdn.microsoft.com/en-us/commandline/wsl/release_notes#build-14951
@@ -47,6 +47,14 @@ const validateHints = (configuration: Configuration) => {
}
};

const validateConnector = (configuration: Configuration) => {
const connectorCofigurationValid = Configuration.validateConnectorConfig(configuration);

if (!connectorCofigurationValid) {
throw new AnalyzerError('Invalid connector configuration', AnalyzerErrorStatus.ConnectorError);
}
};

/**
* Node API.
*/
@@ -95,6 +103,7 @@ export class Analyzer {
const formatters = initFormatters(resources.formatters);

validateResources(resources);
validateConnector(configuration);
validateHints(configuration);

return new Analyzer(configuration, resources, formatters);
@@ -254,6 +254,12 @@ const getAnalyzer = async (userConfig: UserConfig, options: CreateAnalyzerOption
throw e;
}

if (error.status === AnalyzerErrorStatus.ConnectorError) {
logger.error(`Invalid connector configuration in .hintrc`);

throw e;
}

/*
* If the error is not an AnalyzerErrorStatus
* bubble up the exception.
@@ -21,12 +21,15 @@ import browserslist = require('browserslist'); // `require` used because `browse
import mergeWith = require('lodash/mergeWith');

import { debug as d, fs as fsUtils } from '@hint/utils';
import { validate as schemaValidator } from '@hint/utils/dist/src/schema-validation/schema-validator';

import { UserConfig, IgnoredUrl, ConnectorConfig, HintsConfigObject, HintSeverity, CreateAnalyzerOptions } from './types';
import { validateConfig } from './config/config-validator';
import normalizeHints from './config/normalize-hints';
import { validate as validateHint, getSeverity } from './config/config-hints';
import * as resourceLoader from './utils/resource-loader';
import { ResourceType } from './enums';
import { IConnectorConstructor } from './types/connector';

const { isFile, loadJSFile, loadJSONFile} = fsUtils;

@@ -366,6 +369,21 @@ export class Configuration {
return new Configuration(userConfig, browsers, ignoredUrls, hints);
}

public static validateConnectorConfig(config: Configuration) {
const connectorId = config.connector!.name;

debug(`Validating ${connectorId} connector`);

const Connector = resourceLoader.loadResource(connectorId, ResourceType.connector) as IConnectorConstructor;

debug(`Connector schema:`);
debug(Connector.schema);
debug(`User configuration:`);
debug(config.connector!.options!);

return schemaValidator(Connector.schema, config.connector!.options!).valid;
}

/**
* Separate hints based on if the hint configs are valid.
* @param config
@@ -8,6 +8,7 @@ export enum ResourceErrorStatus {
export enum AnalyzerErrorStatus {
AnalyzeError = 'AnalyzeError',
ConfigurationError = 'ConfigurationError',
ConnectorError = 'ConnectorError',
HintError = 'HintError',
ResourceError = 'ResourceError'
}
@@ -8,6 +8,7 @@ import { Engine } from '../engine';

export interface IConnectorConstructor {
new(server: Engine, config?: object, launcher?: ILauncher): IConnector;
schema: any;
}

/** A connector to be used by hint */
@@ -24,6 +24,7 @@ type Configuration = {
fromConfig: () => Configuration;
getFilenameForDirectory: (directory: string) => string;
loadConfigFile: (filePath: string) => UserConfig;
validateConnectorConfig: () => boolean;
validateHintsConfig: () => { invalid: string[]; valid: string[] };
};

@@ -88,6 +89,9 @@ test.beforeEach((t) => {
loadConfigFile(filePath: string): UserConfig {
return {};
},
validateConnectorConfig(): boolean {
return true;
},
validateHintsConfig() {
return {} as any;
}
@@ -161,6 +165,31 @@ test(`If there is any missing or incompatible resource, it should return an erro
t.is(error.status, AnalyzerErrorStatus.ResourceError);
});

test(`If the connector is not configured correctly, it should return an error with the status 'ConnectorError'`, (t) => {
const { Analyzer } = loadScript(t.context);
const sandbox = t.context.sandbox;

const fromConfigStub = sandbox.stub(t.context.configuration.Configuration, 'fromConfig').returns(t.context.configuration.Configuration);
const resourceLoaderStub = sandbox.stub(t.context.resourceLoader, 'loadResources').returns({
connector: null,
formatters: [],
hints: [],
incompatible: [],
missing: [],
parsers: []
});
const validateConnectorConfigStub = sandbox.stub(t.context.configuration.Configuration, 'validateConnectorConfig').returns(false);

const error = t.throws<AnalyzerError>(() => {
Analyzer.create({});
});

t.true(validateConnectorConfigStub.calledOnce);
t.true(resourceLoaderStub.calledOnce);
t.true(fromConfigStub.calledOnce);
t.is(error.status, AnalyzerErrorStatus.ConnectorError);
});

test(`If there is any invalid hint, it should return an error with the status 'HintError'`, (t) => {
const { Analyzer } = loadScript(t.context);
const sandbox = t.context.sandbox;
Oops, something went wrong.

0 comments on commit fa1652b

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