fix: make Contains and collection helpers rune-safe for Unicode strings - #1908
Open
HarshalPatel1972 wants to merge 3 commits into
Open
fix: make Contains and collection helpers rune-safe for Unicode strings#1908HarshalPatel1972 wants to merge 3 commits into
HarshalPatel1972 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates string containment checks in assert.Contains to avoid matching invalid UTF-8 byte fragments that cross Unicode rune boundaries, and adds regression tests covering Unicode-heavy scenarios.
Changes:
- Replace
strings.Containswith a rune-based containment check for string inputs. - Add new helper
runeSliceContainsto compare contiguous rune sub-slices. - Add tests for Unicode behavior in
ContainsandElementsMatch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| assert/assertions.go | Switch string containment to rune-based search and add runeSliceContains helper |
| assert/assertions_test.go | Add Unicode-focused tests for Contains and ElementsMatch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fredbi
added a commit
to go-openapi/testify
that referenced
this pull request
Aug 23, 2026
When the container is a string, containsElement rendered the element with reflect.Value.String(). That returns a placeholder for any kind other than a string, so Contains(t, "hello", 'e') compared "hello" against the literal "<int32 Value>" and failed, naming a type where the character should be. stringContainsElement now dispatches on the element instead: strings.Contains for a string, strings.ContainsRune for a rune, strings.IndexByte for a byte, and the reflect fallback only for a defined string type such as `type Doc string`. Any other kind cannot occur in a string and still reports "does not contain". Left alone: a needle that is not valid UTF-8 still matches on byte boundaries, so Contains(t, "é", "\xa9") stays true, since "\xa9" is the second byte of "é". Upstream converts both operands to []rune to prevent that, which costs an allocation on every string Contains to reject a deliberately malformed needle. reference: github.com/stretchr#1908 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi
added a commit
to go-openapi/testify
that referenced
this pull request
Aug 23, 2026
The August sweep of stretchr/testify turned up four reports describing bugs we had inherited. Each now has a row in the implemented table saying what we adopted and what we left alone, and stretchr#1942 joins stretchr#1940 as the PR behind ErrorNotContains. stretchr#1776 goes in as informational: upstream's generated require documentation lists a bool return that the functions do not have, the same defect our own API pages carried until the signature tables were corrected. Nothing to adopt, but worth recording that both projects found it independently. Two more bugs surfaced while checking those four, neither of them reported upstream, so neither gets a row: InDeltaSlice named expected and actual the wrong way round in its failures, and EqualValues compared a negative signed integer equal to a large unsigned one. Also in this file: the review-frequency line was three months stale, [stretchr#1937] was referenced with no link definition and rendered as literal text, three definitions were duplicated, and the summary counts are recounted from the tables. reference: github.com/stretchr#1776 reference: github.com/stretchr#1874 reference: github.com/stretchr#1875 reference: github.com/stretchr#1898 reference: github.com/stretchr#1899 reference: github.com/stretchr#1908 reference: github.com/stretchr#1931 reference: github.com/stretchr#1940 reference: github.com/stretchr#1942 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes
assert.Containsand underlying collection validation routines rune-safe to prevent false positives and out-of-bounds slicing bugs on multi-byte Unicode strings/emojis.Changes
runeSliceContains, a zero-dependency sliding window helper that evaluates[]runecollections to preserve character boundary integrity.containsElementforreflect.Stringvariants to convert target data into[]runeslices before executing structural matches.assert/assertions_test.go.Motivation
The previous implementation relied directly on
strings.Contains, which works exclusively on raw byte tracking. If a searched token matched a specific sequence of middle-bytes within a multi-byte Unicode rune or emoji, the assertion returned invalid positive outcomes or triggered index runtime panics.Related issues
Closes #1518