Skip to content

Use hex encoding for encodeKeyIfCaseSensitive#3326

Open
steven-tey wants to merge 4 commits intomainfrom
hex-encoding
Open

Use hex encoding for encodeKeyIfCaseSensitive#3326
steven-tey wants to merge 4 commits intomainfrom
hex-encoding

Conversation

@steven-tey
Copy link
Collaborator

@steven-tey steven-tey commented Jan 8, 2026

Summary by CodeRabbit

  • New Features

    • Domain-aware encoding/decoding for link keys so keys on case-sensitive domains are handled reliably.
    • Public helpers to transparently encode/decode keys when needed, improving link stability across domains.
  • Bug Fixes

    • Preserved backward compatibility so existing links continue to work.
    • More consistent link handling to avoid case-related issues.
  • Tests

    • Minor test instrumentation added to aid debugging.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link
Contributor

vercel bot commented Jan 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
dub Ready Ready Preview Jan 8, 2026 3:38pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

Encode/decode for link keys changed from base64 to lowercased hex with an "h:" prefix; decoding now supports both formats. New domain-aware helpers were added to conditionally encode/decode keys and reconstruct links for case-sensitive domains.

Changes

Cohort / File(s) Summary
Link case-sensitivity logic
apps/web/lib/api/links/case-sensitivity.ts
encodeKey() now returns lowercased hex prefixed with h:. decodeKey() recognizes h: (hex, case-insensitive) and falls back to legacy base64. Added isCaseSensitiveDomain, encodeKeyIfCaseSensitive, decodeKeyIfCaseSensitive, and decodeLinkIfCaseSensitive which use CASE_SENSITIVE_DOMAINS to conditionally apply encoding/decoding and reconstruct shortLink paths.
Redirects test
apps/web/tests/redirects/index.test.ts
Minor test change: added console.log({baseUrl: h.baseUrl}) inside the "root" test (no control-flow changes).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • devkiran

Poem

🐰 I hop on keys both old and new,
With "h:" I hum a hexy tune,
Lowercase leaps, legacy stays,
Domains guide my careful ways,
A small rabbit cheers the change 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: implementing hex encoding for the new encodeKeyIfCaseSensitive function to address case-sensitivity issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @apps/web/tests/redirects/index.test.ts:
- Line 19: Remove the debug console.log({baseUrl: h.baseUrl}) line from the test
(index.test.ts): delete that console.log call that references h.baseUrl so the
test output is clean, then run the test suite to confirm no residual debug
logging remains.
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df73241 and 107ef2a.

📒 Files selected for processing (1)
  • apps/web/tests/redirects/index.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

const h = new IntegrationHarness();

test("root", async () => {
console.log({baseUrl: h.baseUrl})
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove debug console.log statement.

This console.log appears to be a debug artifact that should be removed before merging. It clutters test output without providing value in the production test suite.

🧹 Proposed fix
   test("root", async () => {
-    console.log({baseUrl: h.baseUrl})
-
     const response = await fetch(h.baseUrl, fetchOptions);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log({baseUrl: h.baseUrl})
test("root", async () => {
const response = await fetch(h.baseUrl, fetchOptions);
🤖 Prompt for AI Agents
In @apps/web/tests/redirects/index.test.ts at line 19, Remove the debug
console.log({baseUrl: h.baseUrl}) line from the test (index.test.ts): delete
that console.log call that references h.baseUrl so the test output is clean,
then run the test suite to confirm no residual debug logging remains.

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