Skip to content

Fix stringlib against non-ascii chars#184

Open
masaedw wants to merge 3 commits into
moonsharp-devs:legacy/2.0from
masaedw:fix-stringlib
Open

Fix stringlib against non-ascii chars#184
masaedw wants to merge 3 commits into
moonsharp-devs:legacy/2.0from
masaedw:fix-stringlib

Conversation

@masaedw

@masaedw masaedw commented Feb 15, 2017

Copy link
Copy Markdown

Some string functions ignore upper byte of chars of strings.
This causes unexpected behaviors.

ex)

  • "書籍" (U+66F8, U+7C4D) matches "糸筍" (U+7CF8, U+7B4D)
  • "[Aそ]" matches "㍝"

See the test cases.

I confirmed that all test in MoonSharp.Interpreter.Tests.net35-client passed using moonsharp_dev_minimal.sln.

@masaedw masaedw changed the title Fix stringlib Fix stringlib against non-ascii chars Feb 17, 2017
wledfor2 added a commit to wledfor2/moonsharp that referenced this pull request Oct 8, 2017
Eir-nya added a commit to RhenaudTheLukark/CreateYourFrisk that referenced this pull request Sep 4, 2019
Merged the lovely changes from moonsharp-devs/moonsharp#184 into CYF

* Fixed a bug where some `string` library functions would incorrectly match characters that only have the same lower byte as the ones you're trying to match.

Fix by masaedw on GitHub

@LimpingNinja LimpingNinja 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.

Added extra eyes (@Benjamin-Dobell) for review, I ran a few tests and didn't see anything out of the ordinary. Always a bit concerned when removing the byte casting here as it may be the easiest way to fix it, but uncertain (since encoding is not my forte) if this is the right way to fix this.

@LimpingNinja LimpingNinja 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.

Ensure code changes on PR cover the other instances of this while we still have them

@Benjamin-Dobell
Benjamin-Dobell changed the base branch from master to legacy/2.0 March 3, 2026 07:00
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