Skip to content

#3064 - Non-english channel name issue#3086

Open
SebinSong wants to merge 6 commits intomasterfrom
sebin/task/#3064-non-english-channel-name-issue
Open

#3064 - Non-english channel name issue#3086
SebinSong wants to merge 6 commits intomasterfrom
sebin/task/#3064-non-english-channel-name-issue

Conversation

@SebinSong
Copy link
Copy Markdown
Collaborator

@SebinSong SebinSong commented Mar 31, 2026

closes #3064

channel-name-fix.mp4

Open with Devin

@SebinSong SebinSong self-assigned this Mar 31, 2026
@SebinSong
Copy link
Copy Markdown
Collaborator Author

SebinSong commented Mar 31, 2026

/crush_fast


AI review started.

devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 31, 2026

Advanced AI Review

  • Type: Agentic (crush)
  • Model: glm-5.1
Click to expand review

Code Review: Non-English Channel Name Support

Issue 1: .normalize() missing from client-side duplicate checks — pre-existing but now more impactful

  • Addressed
  • Dismissed

Severity: 🟡

With Unicode names now allowed, Unicode normalization collisions become practically relevant. The client-side duplicate checks in both modals lack .normalize(), while the server-side addChatRoom check uses it.

frontend/views/containers/chatroom/CreateNewChannelModal.vue:233:

if (name.toUpperCase() === existingName.toUpperCase()) {

frontend/views/containers/chatroom/EditChannelNameModal.vue:126:

if (name.toUpperCase() === existingName.toUpperCase()) {

vs. frontend/controller/actions/group.js:854:

if (params.data.attributes.name.toUpperCase().normalize() === contractState.chatRooms[contractId].name.toUpperCase().normalize()) {

For example, é (U+00E9) and e + combining accent (U+0065 U+0301) would be treated as different names by the client but identical by the server. A user could pass the client-side check then get a server error.

Suggested fix — both modals should use:

if (name.toUpperCase().normalize() === existingName.toUpperCase().normalize()) {

Issue 2: renameChatRoom has no server-side duplicate name check — pre-existing

  • Addressed
  • Dismissed

Severity: 🟡

While investigating the .normalize() inconsistency, I noticed that renameChatRoom in frontend/controller/actions/group.js:1016 delegates to gi.actions/chatroom/rename with zero duplicate-name validation on the server side. The EditChannelChannelModal.vue client-side check is the sole defense — and it lacks .normalize().

This is pre-existing and not introduced by this PR, but worth noting since the PR touches this exact code path. A malicious or buggy client could rename a channel to duplicate an existing name via the renameChatRoom action.


Issue 3: validateChatRoomName capital-letter check is ASCII-only ([A-Z])

  • Addressed
  • Dismissed

Severity: ⚪️

frontend/model/contracts/shared/functions.js:202:

[L('Chatroom name cannot contain capital letters')]: (v: string): boolean => /[A-Z]/.test(v),

This only detects ASCII capitals. Non-ASCII uppercase letters like É (U+00C9) or Я (U+042F) would pass validation. However, this is not exploitable in practice because sanitizeChannelName calls .toLowerCase() on the input before it reaches contract validation, and .toLowerCase() handles all Unicode. Still, the validator and sanitizer are semantically misaligned — the validator claims "cannot contain capital letters" but only checks ASCII. If you wanted to be precise, you could use:

[L('Chatroom name cannot contain capital letters')]: (v: string): boolean => /\p{Lu}/u.test(v),

Issue 4: maxlength='50' hardcoded instead of using the constant

  • Addressed
  • Dismissed

Severity: ⚪️

frontend/views/containers/chatroom/CreateNewChannelModal.vue:19:

maxlength='50'

The EditChannelNameModal.vue correctly uses a dynamic binding (:maxlength='maxNameCharacters'), but CreateNewChannelModal.vue hardcodes '50'. If the constant CHATROOM_NAME_LIMITS_IN_CHARS ever changes, these would drift apart.

Suggested fix:

:maxlength='maxNameCharacters'

Issue 5: No unit tests for sanitizeChannelName or validateChatRoomName

  • Addressed
  • Dismissed

Severity: ⚪️

There are no tests anywhere in the codebase for either function. Given that this is the core fix for the bug and the regex logic is subtle (Unicode property escapes, interaction between sanitization and validation), adding tests would be valuable. Key cases to cover:

  • Korean/Chinese/Japanese input survives sanitization unchanged
  • Mixed script input (e.g., Hello-한글-123) works
  • Whitespace → hyphen conversion
  • Special characters are stripped
  • .toLowerCase() handles non-ASCII capitals correctly
  • Idempotency: sanitizeChannelName(sanitizeChannelName(x)) === sanitizeChannelName(x)

Review generated using glm-5.1 via Z.AI. Comment /crush_fast to re-run.

@cypress
Copy link
Copy Markdown

cypress bot commented Mar 31, 2026

group-income    Run #5575

Run Properties:  status check passed Passed #5575  •  git commit e9d99bc0ac ℹ️: Merge ba766c87c459520023c31d20dd8c61191a926d8f into b5e795b2c39d98653480dc9a977b...
Project group-income
Branch Review sebin/task/#3064-non-english-channel-name-issue
Run status status check passed Passed #5575
Run duration 14m 25s
Commit git commit e9d99bc0ac ℹ️: Merge ba766c87c459520023c31d20dd8c61191a926d8f into b5e795b2c39d98653480dc9a977b...
Committer Sebin Song
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 10
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 117
View all changes introduced in this branch ↗︎

@SebinSong
Copy link
Copy Markdown
Collaborator Author

SebinSong commented Mar 31, 2026

/crush_fast


AI review started.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 31, 2026

Advanced AI Review

  • Type: Agentic (crush)
  • Model: glm-5.1
Click to expand review

Let me investigate a few more things before giving my review.Now let me check one more thing about how the \p{N} category compares to the old 0-9:Here is my review of the changes.


1. 🟡 \p{N} is overly broad — allows fractions, circled numbers, and superscripts as valid channel name characters

  • Addressed
  • Dismissed

The old code only allowed 0-9. The new \p{N} Unicode category includes \p{Nd} (decimal digits), \p{Nl} (letter numbers like Roman numerals), and \p{No} (other numbers like ½, , ², ³). This means characters like ½, , and ² are now valid in channel names, which is almost certainly unintentional.

frontend/model/contracts/shared/functions.js:206:

[L('Chatroom name cannot contain punctuations or special characters except hyphen')]: (v: string): boolean => /[^\p{L}\p{N}-]/gu.test(v)

frontend/views/utils/filters.js:93:

.replace(/[^\p{L}\p{N}-]/gu, '')

Using \p{Nd} instead of \p{N} would allow decimal digits from all scripts (Arabic-Indic ٠, Devanagari , fullwidth , etc.) while excluding non-digit numbers. This is likely the intended behavior.

Suggested fix — in both locations, replace \p{N} with \p{Nd}:

frontend/model/contracts/shared/functions.js:206:

[L('Chatroom name cannot contain punctuations or special characters except hyphen')]: (v: string): boolean => /[^\p{L}\p{Nd}-]/gu.test(v)

frontend/views/utils/filters.js:93:

    .replace(/[^\p{L}\p{Nd}-]/gu, '')

And update the comment in filters.js accordingly:

      // \p{Nd} - Any decimal digit from any language (0-9, Arabic-Indic, etc.)

2. 🟡 Cross-script homoglyph channel names can bypass duplicate detection

  • Addressed
  • Dismissed

By allowing arbitrary Unicode letters, it becomes possible to create channels whose names are visually indistinguishable from existing ones using characters from different scripts. For example, Cyrillic а (U+0430) looks identical to Latin a (U+0061). The duplicate check in both modals would NOT detect this because:

frontend/views/containers/chatroom/CreateNewChannelModal.vue:235 and frontend/views/containers/chatroom/EditChannelNameModal.vue:128:

if (name.toUpperCase().normalize() === existingName.toUpperCase().normalize()) {

Cyrillic а.toUpperCase() → Cyrillic А (U+0410) ≠ Latin A (U+0041), so the check passes and the duplicate is not detected.

This is a known tradeoff when supporting internationalized identifiers, and full confusable detection is complex. However, two lighter mitigations are worth considering:

  1. Restrict letters to the scripts used by group members' locales (complex, may not be worth it).
  2. At minimum, add a check that rejects names that are confusable with existing names using a simple approach like comparing after stripping all diacritics and normalizing to ASCII via String.prototype.normalize('NFKD') + regex. A basic skeleton:
function toConfusableAscii (str: string): string {
  return str.normalize('NFKD').replace(/[\u0300-\u036f]/g, '').toLowerCase()
}

// In duplicate check:
if (toConfusableAscii(name) === toConfusableAscii(existingName)) {
  return false
}

This won't catch Cyrillic→Latin confusables (since NFKD doesn't map across scripts), but it does help with accented characters. Full confusable detection would require a library like [confusable-json](https://github.com/glampr/confusable-json) or a confusable homoglyph mapping table.


3. 🟡 Watcher modifying v-model input can interfere with IME composition (Vue 2)

  • Addressed
  • Dismissed

Vue 2's v-model fires on every input event, including during IME composition (unlike Vue 3 which waits for compositionend). The watcher immediately overwrites form.name, which updates the input element's value and can reset the IME composition state for some input methods.

frontend/views/containers/chatroom/CreateNewChannelModal.vue:253-259:

watch: {
  'form.name' (newVal) {
    if (newVal.length) {
      this.form.name = sanitizeChannelName(newVal)
    }
  }
}

frontend/views/containers/chatroom/EditChannelNameModal.vue:136-142:

watch: {
  'form.name' (newVal) {
    if (newVal.length) {
      this.form.name = sanitizeChannelName(newVal)
    }
  }
}

Note: the codebase has zero compositionstart/compositionend handlers anywhere (confirmed by search), and this watcher pattern pre-existed this PR. The new code improves the situation because it no longer strips non-English characters, but it could be further improved by guarding against IME composition:

// In component data:
ephemeral: {
  isComposing: false
}

// In template, add to the input element:
@compositionstart='ephemeral.isComposing = true'
@compositionend='ephemeral.isComposing = false'

// In watcher:
'form.name' (newVal) {
  if (newVal.length && !this.ephemeral.isComposing) {
    this.form.name = sanitizeChannelName(newVal)
  }
}

// Also sanitize on compositionend:
'onCompositionEnd' () {
  if (this.form.name.length) {
    this.form.name = sanitizeChannelName(this.form.name)
  }
}

4. ⚪️ Unnecessary g flag on validation regex used with .test()

  • Addressed
  • Dismissed

frontend/model/contracts/shared/functions.js:206:

[L('Chatroom name cannot contain punctuations or special characters except hyphen')]: (v: string): boolean => /[^\p{L}\p{N}-]/gu.test(v)

The g (global) flag has no meaningful effect when used with .test() on a fresh regex literal (since lastIndex starts at 0 each time), but it is unnecessary and could mislead readers into thinking the regex is being used with .match() or .replace(). The u flag is sufficient for Unicode property escapes.

Suggested fix:

[L('Chatroom name cannot contain punctuations or special characters except hyphen')]: (v: string): boolean => /[^\p{L}\p{Nd}-]/u.test(v)

5. ⚪️ Helper text doesn't mention that non-English letters are supported

  • Addressed
  • Dismissed

The whole point of this PR is to support non-English channel names, but the helper text only describes what's not allowed, without indicating that Unicode letters from any language are now accepted.

frontend/views/containers/chatroom/CreateNewChannelModal.vue:28:

i18n.helper.with-icon(v-if='!$v.form.name.$error' tag='p') Channel name can't contain white spaces, special characters, capital letters or punctuations.

frontend/views/containers/chatroom/EditChannelNameModal.vue:24:

i18n.helper.with-icon(v-if='!$v.form.name.$error' tag='p') Channel name can't contain white spaces, special characters, capital letters or punctuations.

A more helpful text would tell users what they can type:

i18n.helper.with-icon(v-if='!$v.form.name.$error' tag='p') Channel names can contain letters from any language, numbers, and hyphens(-).

6. ⚪️ sanitizeChannelName is placed in filters.js but is not a Vue filter

  • Addressed
  • Dismissed

frontend/views/utils/filters.js:59-62:

export const sanitizeChannelName = (name: string): string => {
  return name.replace(/\s/g, '-').toLowerCase()
    .replace(/[^\p{L}\p{N}-]/gu, '')
}

The file filters.js contains Vue filter functions and general display utilities (toPercent, getFileExtension, filterByKeyword). While sanitizeChannelName fits alongside these as a general utility, a more natural home would be a dedicated validation/sanitization utility file, or it could be co-located with validateChatRoomName in frontend/model/contracts/shared/functions.js since they enforce the same rules (one on the client side, one on the contract side). This would also make it easier to keep them in sync if the rules change.


Review generated using glm-5.1 via Z.AI. Comment /crush_fast to re-run.

devin-ai-integration[bot]

This comment was marked as resolved.

@SebinSong
Copy link
Copy Markdown
Collaborator Author

SebinSong commented Apr 1, 2026

/crush_fast {{openrouter, google/gemini-3.1-pro-preview, google/gemini-3-flash-preview}}


AI review started.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

Advanced AI Review

  • Type: Agentic (crush)
  • Model: google/gemini-3.1-pro-preview
Click to expand review

1. 🔴 Unicode Normalization prevents homoglyph spoofing (Security)

  • Addressed
  • Dismissed

The introduction of Unicode classes (\p{L}, \p{M}) into channel names allows both composed (e.g., NFC) and decomposed (e.g., NFD) forms of characters to pass validation. This means "카페" (composed) and "ㅋㅏㅍㅔ" (decomposed presentation) can both be seen as different strings by backend exact-equality checks. To prevent malicious users from bypassing duplicate channel name checks and visually spoofing existing channels with unnormalized or differently normalized strings, NFKC normalization should be applied before cleaning up string characters.

Additionally, using NFKC prevents visual width issues with full-width alphanumeric variants such as full-width digits (like 123) from passing incorrectly undetected or separated.

In frontend/views/utils/filters.js:79-84, you should enforce normalization early in the pipeline pipeline:

export const sanitizeChannelName = (name: string): string => {
  return name.replace(/\s/g, '-') // Replace all whitespace with '-'
    .normalize('NFKC') // Normalize to prevent visual spoofing
    .toLowerCase() // Convert to lowercase
    /*
      The regex below removes everything except:
      \p{L} - Any letter from any language
      \p{M} - Any combining mark
      \p{Nd} - Any digit numbers
      '-'    - The hyphen (separator)

      Reference: Unicode character class escape -
      https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Unicode_character_class_escape
    */
    .replace(/[^\p{L}\p{M}\p{Nd}-]/gu, '')
}

To enforce this safely on the contract level to secure against decentralized API interactions originating outside of the frontend client, introduce a normalization check in frontend/model/contracts/shared/functions.js:195-201:

  const nameValidationMap: {[string]: Function} = {
    [L('Chatroom name must be normalized')]: (v: string): boolean => v !== v.normalize('NFKC'),
    [L('Chatroom name cannot contain white-space')]: (v: string): boolean => /\s/.test(v),
    [L('Chatroom name cannot contain capital letters')]: (v: string): boolean => /\p{Lu}/u.test(v),
    [L('Chatroom name cannot contain punctuation or special characters except hyphens')]: (v: string): boolean => /[^\p{L}\p{M}\p{Nd}-]/u.test(v)
  }

I don't see why "ㅋㅏㅍㅔ" as opposed to "카페" should be seen malicious. It seems people in Korea use these forms pretty commonly in online chat. Dismissed.

2. 🟡 Reconsider how uppercase letters are detected

  • Addressed
  • Dismissed

In frontend/model/contracts/shared/functions.js:198, the regex /\p{Lu}/u checks strictly for characters exclusively in the "Letter, Uppercase" Unicode category. This misses the "Letter, Titlecase" (\p{Lt}) category (e.g., Dz [U+01F2]). If the core requirement is to ensure visually matching lowercase channel names without capital anomalies, testing if a string is simply identical to its lowercased counterpart is a more complete and mathematically sound approach than matching regex groups.

  const nameValidationMap: {[string]: Function} = {
    [L('Chatroom name cannot contain white-space')]: (v: string): boolean => /\s/.test(v),
    [L('Chatroom name cannot contain capital letters')]: (v: string): boolean => v !== v.toLowerCase(),
    [L('Chatroom name cannot contain punctuation or special characters except hyphens')]: (v: string): boolean => /[^\p{L}\p{M}\p{Nd}-]/u.test(v)
  }

/\p{Lu}/u Works just fine.

3. ⚪ Null-safety and simplifications for the duplicate names check

  • Addressed
  • Dismissed

In frontend/views/containers/chatroom/CreateNewChannelModal.vue:234-242 and frontend/views/containers/chatroom/EditChannelNameModal.vue:126-133, .toUpperCase() is being called directly on variables in validation functions. While Vue v-model instances on text fields generally fall back to empty strings rather than null, having programmatic assignments evaluate to null dynamically within a component logic chain evaluates right into Vuelidate checks and crashes the app immediately.

It is also more conventional to case-fold downward using .toLowerCase() (since channel names natively behave as lowercase configurations).

        [L('Duplicate channel name')]: (name, siblings) => {
          if (!name) return true
          const checkName = name.normalize('NFKC').toLowerCase()
          for (const existingName of siblings.existingNames) {
            if (checkName === (existingName || '').normalize('NFKC').toLowerCase()) {
              return false
            }
          }
          return true
        }

name being potentially being null is caught by another validation item above it which is:
[L('This field is required')]: required. So there's no need to make this change. Dismissed.


Review generated using google/gemini-3.1-pro-preview via OpenRouter. Comment /crush_fast {{openrouter, google/gemini-3.1-pro-preview, google/gemini-3-flash-preview}} to re-run.

@SebinSong
Copy link
Copy Markdown
Collaborator Author

The PR is ready.

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.

Cannot create a non-english channel name

2 participants