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.

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

Add rule to check if the web app manifest content is valid #56

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .sonarrc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"lang-attribute": "warning",
"manifest-exists": "warning",
"manifest-file-extension": "warning",
"manifest-is-valid": "warning",
"no-double-slash": "warning"
}
}
9 changes: 9 additions & 0 deletions src/lib/rules/manifest-is-valid/manifest-is-valid.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Disallow web app manifest file with invalid content (manifest-is-valid)

## Rule Details

TODO

## Resources

* [Web App Manifest Specification](https://www.w3.org/TR/appmanifest)
85 changes: 85 additions & 0 deletions src/lib/rules/manifest-is-valid/manifest-is-valid.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* @fileoverview Check if the content of the web app manifest file is valid.
*/

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

import * as url from 'url';

import { Rule, RuleBuilder, ElementFoundEvent } from '../../types'; // eslint-disable-line no-unused-vars
import { RuleContext } from '../../rule-context'; // eslint-disable-line no-unused-vars

const debug = require('debug')('sonar:rules:manifest-is-valid');

// ------------------------------------------------------------------------------
// Public
// ------------------------------------------------------------------------------

const rule: RuleBuilder = {
create(context: RuleContext): Rule {

const manifestIsValid = async (data: ElementFoundEvent) => {
const { element, resource } = data;

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

const manifestHref = element.getAttribute('href');

if (!manifestHref) {
debug(`Web app manifest specified with invalid 'href'`);

return;
}

let manifestURL = '';

// Try to figure out the URL of the web app manifest file.

if (url.parse(manifestHref).protocol) {
manifestURL = manifestHref;
} else {
manifestURL = url.resolve(resource, manifestHref);
}

// Try to get the content of the web app manifest file.

try {
const { response: { body, statusCode } } = await context.fetchContent(manifestURL);

if (statusCode && statusCode !== 200) {
debug(`Web app manifest file could not be fetched (status code: ${statusCode})`);
}

// Validate the content of the web app manifest file.

try {
JSON.parse(body);
// TODO: Add more complex web app manifest file validation.
} catch (e) {
debug('Failed to parse the web app manifest file');
context.report(resource, element, `Web app manifest file doesn't contain valid JSON`);
}

} catch (e) {
debug('Failed to fetch the web app manifest file');
}

}
};

return { 'element::link': manifestIsValid };
},
meta: {
docs: {
category: 'PWA',
description: 'Check if the content of the web app manifest is valid',
recommended: true
},
fixable: 'code',
schema: []
}
};

module.exports = rule;
4 changes: 2 additions & 2 deletions src/tests/lib/rules/manifest-exists/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ const tests: Array<RuleTest> = [
name: `Web app manifest is not specified`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/no-manifest.html')
fixture: path.resolve(__dirname, './fixtures/manifest-not-specified.html')
}, {
name: 'traverse::end',
fixture: path.resolve(__dirname, './fixtures/no-manifest.html')
fixture: path.resolve(__dirname, './fixtures/manifest-not-specified.html')
}],
report: { message: 'Web app manifest not specified' }
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html lang="en">
<head>
<title>test</title>
<link rel="stylesheet" href="style.css">
</head>
<body>

</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html lang="en">
<head>
<title>test</title>
<link rel="manifest" href="https://example.com/site.webmanifest">
</head>
<body>

</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html lang="en">
<head>
<title>test</title>
<link rel="manifest" href="">
</head>
<body>

</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html lang="en">
<head>
<title>test</title>
<link rel="manifest">
</head>
<body>

</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html lang="en">
<head>
<title>test</title>
<link rel="manifest" href="site.webmanifest">
</head>
<body>

</body>
</html>
102 changes: 102 additions & 0 deletions src/tests/lib/rules/manifest-is-valid/tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/* eslint sort-keys: 0, no-undefined: 0 */
import * as path from 'path';

import { Rule } from '../../../../lib/types'; // eslint-disable-line no-unused-vars
import { RuleTest } from '../../../helpers/rule-test-type'; // eslint-disable-line no-unused-vars

import * as ruleRunner from '../../../helpers/rule-runner';
import * as rule from '../../../../lib/rules/manifest-is-valid/manifest-is-valid';

const tests: Array<RuleTest> = [
{
name: `Web app manifest is not specified`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/manifest-not-specified.html')
}]
},
{
name: `Web app manifest is specified with no 'href'`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/manifest-specified-with-no-href.html')
}]
},
{
name: `Web app manifest is specified with empty 'href'`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/manifest-specified-with-empty-href.html')
}]
},
{
name: `Web app manifest is specified and its content is valid JSON`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/manifest-specified.html'),
networkData: [{
request: { headers: null },
response: {
body: '{}',
headers: null,
statusCode: 200
}
}]
}]
},
{
name: `Web app manifest is specified and its content is not valid JSON`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/manifest-specified-as-full-url.html'),
networkData: [{
request: { headers: null },
response: {
body: 'x',
headers: null,
statusCode: 200
}
}]
}],
report: { message: `Web app manifest file doesn't contain valid JSON` }
},
{
name: `Web app manifest is specified as a full URL and its content is valid JSON`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/manifest-specified-as-full-url.html'),
networkData: [{
request: { headers: null },
response: {
body: '{}',
headers: null,
statusCode: 200
}
}]
}]
},
{
name: `Web app manifest is specified and request for file fails`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/manifest-specified.html')
}]
},
{
name: `Web app manifest is specified and request for file fails with status code`,
events: [{
name: 'element::link',
fixture: path.resolve(__dirname, './fixtures/manifest-specified.html'),
networkData: [{
request: { headers: null },
response: {
body: null,
headers: null,
statusCode: 404
}
}]
}]
}
];

ruleRunner.testRule(<Rule>rule, tests);