Skip to content

[Vis Tools] Code Cleanup #5947

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.

Sign up for GitHub

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

Merged
juliawu merged 7 commits into from
Feb 4, 2026
Merged

[Vis Tools] Code Cleanup #5947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
da6565b
Update sanitization with more test cases
juliawu Jan 30, 2026
d42ecba
swap to classname selectors instead of for loops
juliawu Jan 30, 2026
76cc460
cleanup sanitizeSourceUrl
juliawu Jan 30, 2026
93249b8
remove colons from hostname check.
juliawu Jan 30, 2026
61dfc35
respond to viewer comments
juliawu Feb 2, 2026
3211c5c
Merge branch 'master' of https://github.com/datacommonsorg/website in…
juliawu Feb 2, 2026
bcd8932
Merge branch 'master' into comment-followup
juliawu Feb 4, 2026
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
16 changes: 10 additions & 6 deletions static/js/chart/draw_d3_map.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,17 +222,21 @@ function mouseOutAction(
hoverClassNames: string[]
): void {
const container = d3.select(containerElement);
const hoverClassesToRemove = hoverClassNames.join(" ");

// Remove global highlight class
container.classed(HOVER_HIGHLIGHTED_CLASS_NAME, false);

if (placeDcid) {
// Remove highlight class from the specific place
const pathSelection = container.select(`#${getPlacePathId(placeDcid)}`);
for (const className of hoverClassNames) {
pathSelection.classed(className, false);
}
pathSelection.classed(hoverClassesToRemove, false);
} else {
// If no placeDcid is provided, clear all highlights
for (const className of hoverClassNames) {
container.selectAll("." + className).classed(className, false);
}
const allHighlightsSelector = hoverClassNames.map((c) => `.${c}`).join(",");
container
.selectAll(allHighlightsSelector)
.classed(hoverClassesToRemove, false);
}
// bring original highlighted region back to the top
container.select("." + HIGHLIGHTED_CLASS_NAME).raise();
Expand Down
24 changes: 19 additions & 5 deletions static/js/shared/util.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,31 @@ test("getCappedStatVarDate", () => {
describe("sanitizeSourceUrl", () => {
test.each([
// http and https urls should be returned as is
["https://example.com", "https://example.com"],
["http://example.com", "http://example.com"],
["https://example.com", "https://example.com/"],
["http://example.com", "http://example.com/"],
// urls without protocol should be prefixed with https
["example.com", "https://example.com"],
["www.example.com", "https://www.example.com"],
["example.com", "https://example.com/"],
["www.example.com", "https://www.example.com/"],
// whitespace should be handled elegantly
[" example.com ", "https://example.com"],
[" example.com ", "https://example.com/"],
// urls with javascript, vbscript, or data should be sanitized
["javascript:alert(1)", ""],
["vbscript:alert(1)", ""],
["data:text/html,<html></html>", ""],
["https://javascript:alert(1)", ""],
["http://vbscript:alert(1)", ""],
["<html>data:text/html,<html></html>", ""],
// Valid paths with javascript, vbscript, or data should be preserved
[
"https://en.wikipedia.org/wiki/JavaScript",
"https://en.wikipedia.org/wiki/JavaScript",
],
["http://example.com/vbscript/about", "http://example.com/vbscript/about"],
[
"https://my-app.com/api?data=dataPayload",
"https://my-app.com/api?data=dataPayload",
],
["http://javascript.com", "http://javascript.com/"],
// empty string should not result in error
["", ""],
])("should convert %p to %p", (input, expected) => {
Expand Down
28 changes: 14 additions & 14 deletions static/js/shared/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,25 +146,25 @@ export function sanitizeSourceUrl(url: string): string {
if (!url) {
return "";
}

const trimmedUrl = url.trim();
const lowerUrl = trimmedUrl.toLowerCase();

// If it starts with http:// or https://, return as is
if (lowerUrl.startsWith("http://") || lowerUrl.startsWith("https://")) {
return trimmedUrl;
}
// Ensure we have a protocol for the parser to work
// If the input is missing a valid protocol, we prepend https://
// Prepending https:// blocks unsafe protocols like javascript:// or vbscript://
const urlToParse =
trimmedUrl.startsWith("http://") || trimmedUrl.startsWith("https://")
? trimmedUrl
: "https://" + trimmedUrl;

// Block unsafe protocols
if (
lowerUrl.startsWith("javascript:") ||
lowerUrl.startsWith("vbscript:") ||
lowerUrl.startsWith("data:")
) {
try {
const parsed = new URL(urlToParse);
return parsed.href;
} catch (e) {
// If the URL does not have a valid URL structure, return empty
// This will block urls with scripts like http://javascript:alert(1)
return "";
}

// Otherwise, assume it is relative and needs https://
return "https://" + trimmedUrl;
}

/**
Expand Down
Loading