Update domain name truncation to preserve start and end with Unicode ellipsis#15182
Update domain name truncation to preserve start and end with Unicode ellipsis#15182
Conversation
- Changed from showing first 3 parts to showing first 24 + last 24 chars - Use Unicode ellipsis "…" instead of "..." - Applies to domains longer than 48 characters - Updated all related tests in sanitize_content.test.cjs and firewall_blocked_domains.test.cjs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the sanitizeDomainName function to improve domain truncation in firewall logs and URL redaction warnings. Instead of showing only the first 3 domain parts (e.g., a.b.c...), the function now preserves both the beginning and end of long domains, making it easier to identify the TLD and overall domain structure.
Changes:
- Modified truncation logic to show first 24 + last 24 characters for domains exceeding 48 characters
- Replaced ASCII ellipsis
"..."with Unicode ellipsis"…"for proper typographic representation - Updated all test expectations to match the new character-length-based truncation behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| actions/setup/js/sanitize_content_core.cjs | Updated sanitizeDomainName function to use character-length-based truncation (48 char threshold) with Unicode ellipsis |
| actions/setup/js/sanitize_content.test.cjs | Updated test suite with new expectations for domains under/over 48 chars, including comprehensive edge cases |
| actions/setup/js/firewall_blocked_domains.test.cjs | Updated test expectation for multi-part domain to reflect new behavior (no truncation for domains under 48 chars) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it("should truncate domains longer than 48 characters to show first 24 and last 24", () => { | ||
| // This domain is 52 characters long | ||
| const longDomain = "very.long.subdomain.name.with.many.parts.example.com"; | ||
| const result = sanitizeDomainName(longDomain); | ||
| expect(result.length).toBe(49); // 24 + 1 (ellipsis) + 24 | ||
| expect(result).toBe("very.long.subdomain.name…h.many.parts.example.com"); | ||
|
|
||
| // Another long domain test | ||
| expect(sanitizeDomainName("alpha.beta.gamma.delta.epsilon.com")).toBe("alpha.beta.gamma.delta.epsilon.com"); | ||
| }); |
There was a problem hiding this comment.
There are no tests for the exact boundary condition of 48 characters (where the domain should remain unchanged) or 49 characters (where truncation should occur). Consider adding tests like: a 48-character domain that should remain unchanged, and a 49-character domain that should be truncated to verify the boundary behavior is correct.
The
sanitizeDomainNamefunction previously truncated domains by showing only the first 3 parts (e.g.,a.b.c...), losing critical information like the TLD. This made long domain names difficult to identify in firewall logs and URL redaction warnings.Changes
"…"instead of"..."for proper typographic representationExamples
This improves readability in firewall blocked domain reports and URL redaction messages while maintaining the existing alphanumeric+dot sanitization for security.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.