Skip to content

fix(twenty-shared): preserve special characters in URLs#16312

Merged
prastoin merged 5 commits intotwentyhq:mainfrom
asasin235:asasin235-fix-16225-url-encoding
Dec 15, 2025
Merged

fix(twenty-shared): preserve special characters in URLs#16312
prastoin merged 5 commits intotwentyhq:mainfrom
asasin235:asasin235-fix-16225-url-encoding

Conversation

@asasin235
Copy link
Copy Markdown
Contributor

@asasin235 asasin235 commented Dec 4, 2025

Fix: URL Encoding Bug & Code Refactor

Issue

Bug: URLs with encoded characters (e.g., %20 for spaces) were being double-encoded or incorrectly processed in lowercaseUrlOriginAndRemoveTrailingSlash, causing URL mismatches and potential data integrity issues.

Build Error: TS2307: Cannot find module 'src/modules/messaging/message-import-manager/drivers/imap/utils/safe-decode-uri-component.util'

Root Cause Analysis

1. Missing URL Decoding

The lowercaseUrlOriginAndRemoveTrailingSlash function was processing URLs without properly decoding URI components. When URLs contained encoded characters like %20, %2F, etc., they weren't being normalized correctly.

2. Invalid Cross-Package Import

The fix attempted to import safeDecodeURIComponent from twenty-server:

import { safeDecodeURIComponent } from 'src/modules/messaging/message-import-manager/drivers/imap/utils/safe-decode-uri-component.util';

This failed because:

  • The file resides in twenty-shared, a separate package
  • TypeScript cannot resolve internal paths from another package
  • The monorepo uses package exports (twenty-shared/utils) for cross-package imports, not direct file paths

Solution

1. Bug Fix: Added Safe URI Decoding

Updated lowercaseUrlOriginAndRemoveTrailingSlash.ts to properly decode URL components:

export const lowercaseUrlOriginAndRemoveTrailingSlash = (rawUrl: string) => {
  const url = getURLSafely(rawUrl);

  if (!isDefined(url)) {
    return rawUrl;
  }

  const lowercaseOrigin = url.origin.toLowerCase();
  const path =
    safeDecodeURIComponent(url.pathname) + 
    safeDecodeURIComponent(url.search) + 
    url.hash;

  return (lowercaseOrigin + path).replace(/\/$/, '');
};

The safeDecodeURIComponent wrapper handles malformed URI sequences gracefully by returning the original string if decoding fails, preventing runtime crashes.

2. Refactor: Consolidated Shared Utility

Before: Duplicate utility existed in twenty-server

twenty-server/src/modules/messaging/.../utils/safe-decode-uri-component.util.ts

After: Single source of truth in twenty-shared

twenty-shared/src/utils/url/safeDecodeURIComponent.ts

This follows the established pattern in the codebase where shared utilities live in twenty-shared and are imported via subpath exports:

// In twenty-server
import { safeDecodeURIComponent } from 'twenty-shared/utils';

// In twenty-shared (local import)
import { safeDecodeURIComponent } from './safeDecodeURIComponent';

Files Changed

File Action Description
twenty-shared/src/utils/url/safeDecodeURIComponent.ts Created New shared utility (moved from twenty-server)
twenty-shared/src/utils/url/index.ts Modified Added export for safeDecodeURIComponent
twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts Modified Fixed import path, now uses local relative import
twenty-server/.../imap-message-text-extractor.service.ts Modified Updated import to use twenty-shared/utils
twenty-server/.../safe-decode-uri-component.util.ts Deleted Removed duplicate utility

The Utility

// safeDecodeURIComponent.ts
export const safeDecodeURIComponent = (text: string): string => {
  try {
    return decodeURIComponent(text);
  } catch {
    return text;
  }
};

This wrapper is necessary because decodeURIComponent() throws a URIError on malformed sequences (e.g., %E0%A4%A). The safe version returns the original string instead of crashing.

Testing

  • 342 tests passed in twenty-shared
  • lowercaseUrlOriginAndRemoveTrailingSlash.test.ts validates URL normalization behavior
  • No regressions in existing functionality

Impact

  • Bug Fixed: URLs with encoded characters are now properly normalized
  • Code Quality: Eliminated code duplication between packages
  • Maintainability: Single source of truth for URI decoding utility
  • Build: Resolved TS2307 compilation error

The lowercaseUrlOriginAndRemoveTrailingSlash function was encoding
special characters (like accented letters) because URL.pathname
automatically encodes them. This caused URLs like:
  https://test.test/frédéric-destombes-22219837
to become:
  https://test.test/fr%C3%A9d%C3%A9ric-destombes-22219837

Fixed by using decodeURIComponent on pathname and search params.

Fixes twentyhq#16225
Copilot AI review requested due to automatic review settings December 4, 2025 10:23
Comment on lines +12 to +13
const path =
decodeURIComponent(url.pathname) + decodeURIComponent(url.search) + url.hash;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: decodeURIComponent() calls in lowercaseUrlOriginAndRemoveTrailingSlash lack error handling, leading to server crashes for malformed URLs.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The lowercaseUrlOriginAndRemoveTrailingSlash function uses decodeURIComponent() on url.pathname and url.search without error handling. decodeURIComponent() throws a URIError for malformed percent-encoding (e.g., %invalid). The URL constructor accepts such malformed URLs, allowing url.pathname to contain sequences that will cause decodeURIComponent() to throw. Since the function processes user-provided URLs from APIs, a malformed URL like https://example.com/path%invalid will lead to an uncaught URIError, crashing the server.

💡 Suggested Fix

Replace direct calls to decodeURIComponent() with the existing safeDecodeURIComponent() utility to gracefully handle malformed percent-encoding.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts#L12-L13

Potential issue: The `lowercaseUrlOriginAndRemoveTrailingSlash` function uses
`decodeURIComponent()` on `url.pathname` and `url.search` without error handling.
`decodeURIComponent()` throws a `URIError` for malformed percent-encoding (e.g.,
`%invalid`). The `URL` constructor accepts such malformed URLs, allowing `url.pathname`
to contain sequences that will cause `decodeURIComponent()` to throw. Since the function
processes user-provided URLs from APIs, a malformed URL like
`https://example.com/path%invalid` will lead to an uncaught `URIError`, crashing the
server.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5455160

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Dec 4, 2025

Greptile Overview

Greptile Summary

This PR fixes URL encoding issues where special characters (accented letters) were being percent-encoded when saved via REST or GraphQL APIs. The fix decodes pathname and search parameters to preserve the original characters.

Key Changes:

  • Modified lowercaseUrlOriginAndRemoveTrailingSlash to decode pathname and search params using decodeURIComponent
  • Added test cases for URLs with special characters (both raw and pre-encoded)

Critical Issue:

  • decodeURIComponent can throw URIError on malformed percent-encoded strings (e.g., %2 or %test), which will cause runtime errors. The codebase already has a safeDecodeURIComponent utility that wraps this with try-catch to handle such cases safely.

Confidence Score: 2/5

  • This PR has a critical bug that can cause runtime errors on malformed URLs
  • The implementation uses unsafe decodeURIComponent which throws exceptions on malformed input. This is a known issue in the codebase (evidenced by the existing safeDecodeURIComponent utility), and the fix should use error-safe decoding to prevent crashes
  • Pay close attention to packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts - needs error handling for decodeURIComponent

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts 2/5 Added decodeURIComponent to preserve special characters, but missing error handling for malformed URLs
packages/twenty-shared/src/utils/url/tests/lowercaseUrlOriginAndRemoveTrailingSlash.test.ts 4/5 Added comprehensive test cases for special characters, but missing edge case tests for malformed percent-encoding

Sequence Diagram

sequenceDiagram
    participant API as REST/GraphQL API
    participant Transform as transformLinksValue
    participant Util as lowercaseUrlOriginAndRemoveTrailingSlash
    participant URL as URL Parser
    
    API->>Transform: Save URL with special chars<br/>(e.g., "frédéric")
    Transform->>Util: Process URL
    Util->>URL: new URL(rawUrl)
    Note over URL: Automatically encodes<br/>pathname to "%C3%A9"
    URL-->>Util: URL object with encoded pathname
    Util->>Util: decodeURIComponent(pathname)
    Note over Util: ⚠️ Can throw URIError<br/>on malformed encoding
    Util->>Util: decodeURIComponent(search)
    Util-->>Transform: Decoded URL with "frédéric"
    Transform-->>API: Store decoded URL
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

const lowercaseOrigin = url.origin.toLowerCase();
const path = url.pathname + url.search + url.hash;
const path =
decodeURIComponent(url.pathname) + decodeURIComponent(url.search) + url.hash;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: decodeURIComponent throws URIError on malformed percent-encoded strings. The codebase already has safeDecodeURIComponent in packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/safe-decode-uri-component.util.ts that handles this.

Suggested change
decodeURIComponent(url.pathname) + decodeURIComponent(url.search) + url.hash;
safeDecodeURIComponent(url.pathname) + safeDecodeURIComponent(url.search) + url.hash;

Consider moving safeDecodeURIComponent to twenty-shared/utils and using it here to prevent runtime errors on malformed URLs.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts
Line: 13:13

Comment:
**logic:** `decodeURIComponent` throws `URIError` on malformed percent-encoded strings. The codebase already has `safeDecodeURIComponent` in `packages/twenty-server/src/modules/messaging/message-import-manager/drivers/imap/utils/safe-decode-uri-component.util.ts` that handles this.

```suggestion
    safeDecodeURIComponent(url.pathname) + safeDecodeURIComponent(url.search) + url.hash;
```

Consider moving `safeDecodeURIComponent` to `twenty-shared/utils` and using it here to prevent runtime errors on malformed URLs.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an issue where URLs with special characters (like accented letters) were being unintentionally percent-encoded when saved via REST or GraphQL APIs. The fix decodes the pathname and search parameters to preserve the original special characters.

Key Changes:

  • Modified lowercaseUrlOriginAndRemoveTrailingSlash to decode pathname and search parameters using decodeURIComponent()
  • Added test cases to verify special characters are preserved in paths and query parameters

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/twenty-shared/src/utils/url/lowercaseUrlOriginAndRemoveTrailingSlash.ts Added decodeURIComponent() calls to pathname and search parameters to preserve special characters
packages/twenty-shared/src/utils/url/__tests__/lowercaseUrlOriginAndRemoveTrailingSlash.test.ts Added test cases for URLs with special characters in paths and query parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +12 to +13
const path =
decodeURIComponent(url.pathname) + decodeURIComponent(url.search) + url.hash;
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The decodeURIComponent() calls can throw a URIError if the URL contains malformed percent-encoding sequences (e.g., %E0%A4%A - incomplete UTF-8 sequence). This should be wrapped in a try-catch block to prevent the application from crashing. Consider falling back to the original encoded values if decoding fails.

Example fix:

const decodeSafely = (str: string) => {
  try {
    return decodeURIComponent(str);
  } catch {
    return str;
  }
};
const path = decodeSafely(url.pathname) + decodeSafely(url.search) + url.hash;

Copilot uses AI. Check for mistakes.
Joker added 2 commits December 4, 2025 16:11
…ge import

- Created safeDecodeURIComponent utility in twenty-shared/utils/url
- Fixed broken import in lowercaseUrlOriginAndRemoveTrailingSlash.ts that was
  incorrectly importing from twenty-server's internal path
- Updated twenty-server's imap-message-text-extractor.service.ts to use the
  shared utility from twenty-shared/utils
- Removed duplicate utility from twenty-server

This consolidates the safeDecodeURIComponent function into the shared package,
following the existing pattern for cross-package utility sharing.
- Malformed percent-encoding (incomplete sequences like %E0%A4%A)
- Double-encoded URLs (%2520 -> %20)
- Special characters in hash fragments (URL.hash encodes them)
- Mixed encoded/non-encoded characters in same URL
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 4, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:8259

This environment will automatically shut down when the PR is closed or after 5 hours.

Copy link
Copy Markdown
Contributor

@prastoin prastoin left a comment

Choose a reason for hiding this comment

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

Hey there, indeed nice catch !
Is this by any chance related to any existing issue ?
We should add an twenty-server integration test that will create a record with special char in a field link to ensure the whole chain works as expected

@prastoin prastoin self-assigned this Dec 10, 2025

const lowercaseOrigin = url.origin.toLowerCase();
const path = url.pathname + url.search + url.hash;
const path =
Copy link
Copy Markdown
Contributor

@prastoin prastoin Dec 10, 2025

Choose a reason for hiding this comment

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

Remark: We don't want inserting null values here
We should not use the safe here but throw in case the link is invalid by definition
This requires a wider refactor checking that all callers of this method do validate the link before being passed to this method

Nevermind double checked the safeDecodeURIComponent implem and it's returning the text

export const safeDecodeURIComponent = (text: string): string => {
  try {
    return decodeURIComponent(text);
  } catch {
    return text;
  }
};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for double-checking! Yes, it falls back to the original text if decoding fails.

Can you plrase approve my PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@prastoin please Approve and merge

Copy link
Copy Markdown
Contributor

@prastoin prastoin left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution !

@prastoin prastoin enabled auto-merge (squash) December 15, 2025 08:35
@prastoin prastoin merged commit 1119e3d into twentyhq:main Dec 15, 2025
80 of 82 checks passed
@twenty-eng-sync
Copy link
Copy Markdown

Hey @prastoin! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

@github-actions
Copy link
Copy Markdown
Contributor

Thanks @asasin235 for your contribution!
This marks your 1st PR on the repo. You're top 38% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants