Skip to content

fix(gti-mcp): build URL ids with URL-safe base64 - #342

Merged
dandye merged 1 commit into
google:mainfrom
VISHNU0906:fix/gti-url-id-urlsafe
Nov 9, 2026
Merged

dandye merged 1 commit into
google:mainfrom
VISHNU0906:fix/gti-url-id-urlsafe

Conversation

@VISHNU0906

Copy link
Copy Markdown
Contributor

get_url_report and get_entities_related_to_an_url build the URL id with standard base64. The API wants URL-safe base64 without padding, which is what vt.url_id does, so a URL whose encoding contains / or + is looked up under the wrong id. A / also splits the id into extra path segments. URLs with a query string hit this a lot. For example, https://example.com/?a=1 encodes to aHR0cHM6Ly9leGFtcGxlLmNvbS8/YT0x, so the request goes to /urls/aHR0cHM6Ly9leGFtcGxlLmNvbS8/YT0x instead of /urls/aHR0cHM6Ly9leGFtcGxlLmNvbS8_YT0x.

The fix switches url_to_base64 to base64.urlsafe_b64encode. URLs that never produced / or + get the same id as before.

Tests:

  • test_get_reports and test_get_entities_related have new URL cases whose standard encoding contains / and +. They go through the MCP client and the local HTTP server, so they check the exact path requested.
  • test_url_to_base64_matches_vt_url_id compares the helper with vt.url_id.

With the old encoder, those 4 new cases fail. The full gti suite passes (67 passed).

cc @dandye

get_url_report and get_entities_related_to_an_url encoded the URL with
standard base64. The API expects the URL-safe alphabet (vt.url_id), so any
URL whose encoding contains "/" or "+" was looked up under the wrong id,
and a "/" also split the id into extra path segments. URLs with query
strings hit this often, e.g. https://example.com/?a=1.

Signed-off-by: KVR Vishnu <46535652+VISHNU0906@users.noreply.github.com>
@VISHNU0906
VISHNU0906 requested a review from a team November 9, 2026 07:45

@dandye dandye left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. LGTM, merging now.

@dandye
dandye merged commit 6aec294 into google:main Nov 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants