[Vis Tools] Code Cleanup #5947

juliawu · 2026-01-30T22:50:22Z

This PR addresses reviewer comments from #5933 and #5934 that I missed addressing before they got merged. The changes are:

Use CSS selector syntax instead of for loops

In d3 map drawing, switch to using pure CSS selectors instead of for loops for better performance, following a Gemini review suggestion
Per reviewer comment, adjusted the gemini suggestion to improve readability of the map/joins used to manipulate the CSS selectors

Adjust the URL sanitization util used by the metadata modal

Per reviewer comment, adds some new test cases to flesh out the unit test
Updates sanitizationSourceUrl so that new test cases pass

gemini-code-assist · 2026-01-30T22:50:40Z

Summary of Changes

Hello @juliawu, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing code quality and security. It optimizes D3 map rendering by transitioning from iterative class removal to more efficient CSS selector-based operations and significantly strengthens the URL sanitization utility to better protect against various forms of malicious URL inputs, incorporating feedback from prior reviews.

Highlights

D3 Map Performance Optimization: Refactored the mouseOutAction function in static/js/chart/draw_d3_map.ts to replace for loops with D3's CSS selector syntax for removing hover classes. This change improves rendering performance and aligns with D3 best practices for DOM manipulation.
Enhanced URL Sanitization Security: Improved the sanitizeSourceUrl utility in static/js/shared/util.ts by leveraging the native URL API for more robust parsing and validation. New checks were added to detect and block malicious protocols and script injection attempts within the hostname, complemented by a comprehensive set of new test cases in static/js/shared/util.test.ts.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request includes some nice cleanups. The refactoring in draw_d3_map.ts to use D3's CSS selector capabilities instead of loops is a great improvement for both performance and code clarity. The update to sanitizeSourceUrl to use the URL API is also a good move towards a more robust implementation. I've found a potential security issue in the new sanitization logic where it might not correctly block all malicious URLs, and I've left a comment with a suggested fix. The new test cases were very helpful in identifying this.

gemini-code-assist · 2026-01-30T22:52:10Z

static/js/shared/util.ts

+    const blockedKeywords = ["javascript:", "vbscript:", "data:"];
+
+    // Block unsafe protocols
+    if (blockedKeywords.includes(parsed.protocol.toLowerCase())) {
+      return "";
+    }
+
+    // Check for script injection in the HOSTNAME
+    if (blockedKeywords.includes(parsed.hostname.toLowerCase())) {
+      return "";
+    }


The current implementation has a potential security flaw. The blockedKeywords array includes colons, which means the check against parsed.hostname will not work as intended. For example, for the URL http://vbscript:alert(1), parsed.hostname is vbscript, and blockedKeywords.includes('vbscript') will be false, allowing the URL to pass. To fix this, I suggest removing the colons from blockedKeywords and adjusting the protocol check accordingly to correctly block unsafe hostnames.

Suggested change

const blockedKeywords = ["javascript:", "vbscript:", "data:"];

// Block unsafe protocols

if (blockedKeywords.includes(parsed.protocol.toLowerCase())) {

return "";

}

// Check for script injection in the HOSTNAME

if (blockedKeywords.includes(parsed.hostname.toLowerCase())) {

return "";

}

const blockedKeywords = ["javascript", "vbscript", "data"];

// Block unsafe protocols

if (blockedKeywords.includes(parsed.protocol.slice(0, -1).toLowerCase())) {

return "";

}

// Check for script injection in the HOSTNAME

if (blockedKeywords.includes(parsed.hostname.toLowerCase())) {

return "";

}

The current code correctly blocks the "http://vbscript:alert(1)" test case, but I've updated the test to explicitly look for hostnames without colons.

beets

thanks for the updates

beets · 2026-01-31T01:05:17Z

static/js/shared/util.ts

+    const blockedHostnames = ["javascript", "vbscript", "data"];
+    if (blockedHostnames.includes(parsed.hostname.toLowerCase())) {
+      return "";
+    }


this block isn't necessary, since those won't parse if invalid

javascript.com is fine, javascript: isn't. the latter will throw a parse error

Good call out, removed this block.

Also added javascript.com as a test case

beets · 2026-01-31T01:07:16Z

static/js/shared/util.ts

+    const blockedProtocols = ["javascript:", "vbscript:", "data:"];
+    if (blockedProtocols.includes(parsed.protocol.toLowerCase())) {
+      return "";
+    }


maybe the reverse instead: check for http and https only, return blank in all other cases.

but, this is already done above though (155-158) so unnecessary to do again here.

Good call out, removed this block and added a clarifying comment so the implicit protocol block mechanism is more clear

beets · 2026-01-31T01:07:40Z

static/js/shared/util.ts

+    trimmedUrl.toLowerCase().startsWith("http://") ||
+    trimmedUrl.toLowerCase().startsWith("https://")
+      ? trimmedUrl
+      : "https://" + trimmedUrl;


avoid calling toLowercase so many times throughout the function

Now that the explicit checks are removed, toLowercase is no longer needed. Removed.

…to comment-followup

juliawu · 2026-02-02T18:14:46Z

@beets thanks for the comments, I think the sanitization function is much cleaner now!

juliawu added 3 commits January 30, 2026 14:03

Update sanitization with more test cases

da6565b

swap to classname selectors instead of for loops

d42ecba

cleanup sanitizeSourceUrl

76cc460

juliawu requested a review from beets January 30, 2026 22:50

gemini-code-assist bot reviewed Jan 30, 2026

View reviewed changes

remove colons from hostname check.

93249b8

beets reviewed Jan 31, 2026

View reviewed changes

juliawu added 2 commits February 2, 2026 10:12

respond to viewer comments

61dfc35

Merge branch 'master' of https://github.com/datacommonsorg/website in…

3211c5c

…to comment-followup

juliawu requested a review from beets February 2, 2026 18:16

[Vis Tools] Code Cleanup #5947

Are you sure you want to change the base?

[Vis Tools] Code Cleanup #5947

Uh oh!

Conversation

juliawu commented Jan 30, 2026

Uh oh!

gemini-code-assist bot commented Jan 30, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beets left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juliawu commented Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants