Commit 425f7c2b authored by Robert Knight's avatar Robert Knight

Handle invalid URIs gracefully in document domain extraction

Since h has very liberal URI parsing compared to the browser's URL
constructor, as well as some completely invalid data on old annotations,
the client needs to handle errors parsing annotation `uri` values.

Surprisingly `annotation-metadata.js` is the only place I found in the
client that attempts to do this, everywhere else is just using the `uri`
field as a string.

Fixes https://github.com/hypothesis/client/issues/3666
parent fffd5d87
...@@ -6,25 +6,34 @@ ...@@ -6,25 +6,34 @@
/** @typedef {import('../../types/api').TextPositionSelector} TextPositionSelector */ /** @typedef {import('../../types/api').TextPositionSelector} TextPositionSelector */
/** @typedef {import('../../types/api').TextQuoteSelector} TextQuoteSelector */ /** @typedef {import('../../types/api').TextQuoteSelector} TextQuoteSelector */
/** Extract a URI, domain and title from the given domain model object. /**
* Extract document metadata from an annotation.
* *
* @param {Annotation} annotation * @param {Annotation} annotation
*
*/ */
export function documentMetadata(annotation) { export function documentMetadata(annotation) {
const uri = annotation.uri; const uri = annotation.uri;
let domain = new URL(uri).hostname; let domain;
let title = domain; try {
domain = new URL(uri).hostname;
if (annotation.document && annotation.document.title) { } catch {
title = annotation.document.title[0]; // Annotation URI parsing on the backend is very liberal compared to the URL
// constructor. There is also some historic invalid data in h (eg [1]).
// Hence we must handle URL parsing failures in the client.
//
// [1] https://github.com/hypothesis/client/issues/3666
domain = '';
} }
if (domain === 'localhost') { if (domain === 'localhost') {
domain = ''; domain = '';
} }
let title = domain;
if (annotation.document && annotation.document.title) {
title = annotation.document.title[0];
}
return { return {
uri, uri,
domain, domain,
......
...@@ -51,6 +51,14 @@ describe('sidebar/helpers/annotation-metadata', () => { ...@@ -51,6 +51,14 @@ describe('sidebar/helpers/annotation-metadata', () => {
assert.equal(documentMetadata(annotation).title, 'example.com'); assert.equal(documentMetadata(annotation).title, 'example.com');
}); });
}); });
['http://localhost:5000', '[not a URL]'].forEach(uri => {
it('returns empty domain if URL is invalid or private', () => {
const annotation = fakeAnnotation({ uri });
const { domain } = documentMetadata(annotation);
assert.equal(domain, '');
});
});
}); });
context('when the annotation does not have a document property', () => { context('when the annotation does not have a document property', () => {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment