Skip to content

Commit

Permalink
invalid external URLs should be considered broken (mdn#3699)
Browse files Browse the repository at this point in the history
* invalid external URLs should be considered broken

Fixes mdn#3698

* eslint fix

* prettier
  • Loading branch information
peterbe committed Jun 1, 2021
1 parent f583354 commit a84c31e
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 3 deletions.
13 changes: 12 additions & 1 deletion build/flaws/broken-links.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,18 @@ function getBrokenLinksFlaws(doc, $, { rawContent }, level) {
}

if (href.startsWith("http://")) {
const domain = new URL(href).hostname;
let domain = null;
try {
domain = new URL(href).hostname;
} catch (err) {
return addBrokenLink(
a,
checked.get(href),
href,
null,
"Not a valid link URL"
);
}
// If a URL's domain is in the list that getSafeToHttpDomains() provides,
// that means we've tested that you can turn that into a HTTPS link
// simply by replacing the `http://` for `https://`.
Expand Down
6 changes: 4 additions & 2 deletions client/src/document/toolbar/flaws.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -484,12 +484,14 @@ function BrokenLinks({
{flaw.fixable && <FixableFlawBadge />}{" "}
{opening && opening === key && <span>Opening...</span>}
<br />
{flaw.suggestion && (
{flaw.suggestion ? (
<span>
<b>Suggestion:</b>
<ShowDiff before={flaw.href} after={flaw.suggestion} />
</span>
)}{" "}
) : (
<code>{flaw.explanation}</code>
)}
</li>
);
})}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
title: Http:// link that is not a valid link
slug: Web/BrokenLinks/Broken_http_link
---

<p><a href="http://"><code>http://</code></a></p>
21 changes: 21 additions & 0 deletions testing/tests/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,27 @@ test("repeated broken links flaws", () => {
expect(map.get("link3").suggestion).toBe("/en-US/docs/Web/CSS/number");
});

test("broken http:// link that is not a valid URL", () => {
const builtFolder = path.join(
buildRoot,
"en-us",
"docs",
"web",
"brokenlinks",
"broken_http_link"
);
const jsonFile = path.join(builtFolder, "index.json");
const { doc } = JSON.parse(fs.readFileSync(jsonFile));
const { flaws } = doc;
expect(flaws.broken_links.length).toBe(1);

const map = new Map(flaws.broken_links.map((x) => [x.id, x]));
expect(map.size).toBe(1);
expect(map.get("link1").suggestion).toBeNull();
expect(map.get("link1").explanation).toBe("Not a valid link URL");
expect(map.get("link1").fixable).toBeFalsy();
});

test("without locale prefix broken links flaws", () => {
// This fixture has the same broken link, that redirects, 3 times.
const builtFolder = path.join(
Expand Down

0 comments on commit a84c31e

Please sign in to comment.