Skip to content

Potential fix for code scanning alert no. 10: String length conflation - #756

Merged
tianjianjiang merged 2 commits into
masterfrom
alert-autofix-10
Dec 5, 2025
Merged

Potential fix for code scanning alert no. 10: String length conflation#756
tianjianjiang merged 2 commits into
masterfrom
alert-autofix-10

Conversation

@tianjianjiang

@tianjianjiang tianjianjiang commented Dec 5, 2025

Copy link
Copy Markdown
Member

User description

Potential fix for https://github.com/openvanilla/McBopomofo/security/code-scanning/10

To fix the problem, ensure that all offsets and lengths passed to NSRange (such as in NSMakeRange) are expressed in UTF-16 code units as expected by NSString and Cocoa APIs. Specifically, instead of using composingBuffer.count, use the length of the corresponding NSString, i.e. NSString(string: composingBuffer).length.
File/region to change:

  • File: Source/InputMethodController.swift
  • Line(s): 724
    Update the selectionRange: NSMakeRange(composingBuffer.count, 0) to use the NSString length instead.
    No new methods or complex imports are required; NSString is available via Foundation/Cocoa.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.


PR Type

Bug fix


Description

  • Fix UTF-16 length for NSRange

  • Prevent string length conflation issue

  • Align selectionRange with Cocoa expectations


Diagram Walkthrough

flowchart LR
  str["Swift String length (characters)"]
  utf16["NSString length (UTF-16 units)"]
  nsrange["NSRange for setMarkedText"]
  apis["Cocoa Text APIs"]

  str -- "was used (incorrect)" --> nsrange
  utf16 -- "now used (correct)" --> nsrange
  nsrange -- "compatible with" --> apis
Loading

File Walkthrough

Relevant files
Bug fix
InputMethodController.swift
Use NSString length for NSRange in setMarkedText                 

Source/InputMethodController.swift

  • Replace character count with NSString length.
  • Build NSRange using UTF-16 code units.
  • Update setMarkedText selectionRange argument.
+1/-1     

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@gemini-code-assist

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@tianjianjiang
tianjianjiang marked this pull request as ready for review December 5, 2025 07:50
Copilot AI review requested due to automatic review settings December 5, 2025 07:50
gemini-code-assist[bot]

This comment was marked as resolved.

@github-actions

github-actions Bot commented Dec 5, 2025

Copy link
Copy Markdown

Persistent review updated to latest commit a211050

This comment was marked as resolved.

@lukhnos lukhnos left a comment

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.

Let's take the Copilot's suggestion, i.e. replace composingBuffer.count with composingBuffer.utf16.count, which is the correct idiomatic fix here.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@lukhnos lukhnos left a comment

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.

LGTM. Thanks!

@tianjianjiang
tianjianjiang merged commit b333b92 into master Dec 5, 2025
16 checks passed
@tianjianjiang
tianjianjiang deleted the alert-autofix-10 branch December 5, 2025 19:21
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