-
Notifications
You must be signed in to change notification settings - Fork 9.6k
core(manifest-parser): handle blob manifests #9088
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -151,7 +151,7 @@ function parseStartUrl(jsonInput, manifestUrl, documentUrl) { | |
| return { | ||
| raw, | ||
| value: documentUrl, | ||
| warning: 'ERROR: invalid start_url relative to ${manifestUrl}', | ||
| warning: `ERROR: invalid start_url relative to ${manifestUrl}`, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -229,8 +229,14 @@ function parseIcon(raw, manifestUrl) { | |
| src.value = undefined; | ||
| } | ||
| if (src.value) { | ||
| // 9.4(4) - construct URL with manifest URL as the base | ||
| src.value = new URL(src.value, manifestUrl).href; | ||
| try { | ||
| // 9.4(4) - construct URL with manifest URL as the base | ||
| src.value = new URL(src.value, manifestUrl).href; | ||
| } catch (_) { | ||
| // 9.6 "This algorithm will return a URL or undefined." | ||
patrickhulce marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| src.warning = `ERROR: invalid icon url will be ignored ${raw.src}`; | ||
patrickhulce marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| src.value = undefined; | ||
| } | ||
| } | ||
|
|
||
| const type = parseString(raw.type, true); | ||
|
|
@@ -333,7 +339,7 @@ function parseApplication(raw) { | |
| appUrl.value = new URL(appUrl.value).href; | ||
| } catch (e) { | ||
| appUrl.value = undefined; | ||
| appUrl.warning = 'ERROR: invalid application URL ${raw.url}'; | ||
| appUrl.warning = `ERROR: invalid application URL ${raw.url}`; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -460,10 +466,21 @@ function parse(string, manifestUrl, documentUrl) { | |
| }; | ||
| /* eslint-enable camelcase */ | ||
|
|
||
| /** @type {string|undefined} */ | ||
| let manifestUrlWarning; | ||
| try { | ||
| const manifestUrlParsed = new URL(manifestUrl); | ||
| if (!manifestUrlParsed.protocol.startsWith('http')) { | ||
| manifestUrlWarning = `WARNING: manifest URL not available over a valid network protocol`; | ||
| } | ||
| } catch (_) { | ||
| manifestUrlWarning = `ERROR: invalid manifest URL ${manifestUrl}`; | ||
|
||
| } | ||
|
|
||
| return { | ||
| raw: string, | ||
| value: manifest, | ||
| warning: undefined, | ||
| warning: manifestUrlWarning, | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ const manifestStub = require('../fixtures/manifest.json'); | |
|
|
||
| const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json'; | ||
| const EXAMPLE_DOC_URL = 'https://example.com/index.html'; | ||
| const EXAMPLE_MANIFEST_BLOB_URL = 'blob:https://example.com/manifest.json'; | ||
|
|
||
| /** | ||
| * Simple manifest parsing helper when the manifest URLs aren't material to the | ||
|
|
@@ -47,6 +48,16 @@ describe('Manifest Parser', function() { | |
| // prefer_related_applications | ||
| }); | ||
|
|
||
| it('should warn on invalid manifest parser URL', function() { | ||
| const parsedManifest = manifestParser('{}', 'not a URL', EXAMPLE_DOC_URL); | ||
| assert.ok(parsedManifest.warning); | ||
| }); | ||
|
|
||
| it('should warn on valid but non-HTTPS manifest parser URL', function() { | ||
patrickhulce marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const parsedManifest = manifestParser('{}', EXAMPLE_MANIFEST_BLOB_URL, EXAMPLE_DOC_URL); | ||
| assert.ok(parsedManifest.warning); | ||
|
||
| }); | ||
|
|
||
| describe('icon parsing', function() { | ||
| // 9.7 | ||
| it('gives an empty array and an error for erroneous icons entry', () => { | ||
|
|
@@ -110,6 +121,18 @@ describe('Manifest Parser', function() { | |
| assert.equal(icons.value.length, 0); | ||
| }); | ||
|
|
||
| it('parses icons and discards any with invalid base URL values', () => { | ||
| const manifestSrc = JSON.stringify({ | ||
| icons: [{ | ||
| src: '/valid/path', | ||
| }], | ||
| }); | ||
| const parsedManifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_BLOB_URL, | ||
| EXAMPLE_DOC_URL); | ||
| const icons = parsedManifest.value.icons; | ||
| assert.equal(icons.value.length, 0); | ||
|
||
| }); | ||
|
|
||
| it('parses icons and discards any with undefined or empty string src values', () => { | ||
| const manifestSrc = JSON.stringify({ | ||
| icons: [{ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol, that's certainly one big problem with testing via
assert.ok(parsedUrl.warning)...