Skip to content

Ignore match strings past the first NUL rune#24

Merged
sahilm merged 3 commits into
sahilm:masterfrom
nfreya:bugfix/panic-on-nuls-in-data
Aug 2, 2025
Merged

Ignore match strings past the first NUL rune#24
sahilm merged 3 commits into
sahilm:masterfrom
nfreya:bugfix/panic-on-nuls-in-data

Conversation

@nfreya

@nfreya nfreya commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Not doing so can lead to patternIndex exceeding the bounds of runes.

Not doing so can lead to patternIndex exceeding the bounds of runes.

@sahilm sahilm left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the PR. Please add a test that demonstrates the bug.

@nfreya

nfreya commented Jul 31, 2025

Copy link
Copy Markdown
Contributor Author

Trace from the new test running without the patch:

--- FAIL: TestFindWithCannedData (0.00s)
panic: runtime error: index out of range [2] with length 2 [recovered]
	panic: runtime error: index out of range [2] with length 2

goroutine 8 [running]:
testing.tRunner.func1.2({0x587360, 0xc000018420})
	/usr/lib/go/src/testing/testing.go:1734 +0x21c
testing.tRunner.func1()
	/usr/lib/go/src/testing/testing.go:1737 +0x35e
panic({0x587360?, 0xc000018420?})
	/usr/lib/go/src/runtime/panic.go:792 +0x132
github.com/sahilm/fuzzy.FindFromNoSort({0x597623?, 0x65?}, {0x5d5df0, 0xc000010ae0})
	/home/go/fuzzy/fuzzy.go:136 +0x945
github.com/sahilm/fuzzy.FindFrom({0x597623?, 0x1?}, {0x5d5df0?, 0xc000010ae0?})
	/home/go/fuzzy/fuzzy.go:99 +0x25
github.com/sahilm/fuzzy.Find(...)
	/home/go/fuzzy/fuzzy.go:83
github.com/sahilm/fuzzy_test.TestFindWithCannedData(0xc0000d2700)
	/home/go/fuzzy/fuzzy_test.go:115 +0x81e
testing.tRunner(0xc0000d2700, 0x5a57d0)
	/usr/lib/go/src/testing/testing.go:1792 +0xf4
created by testing.(*T).Run in goroutine 1
	/usr/lib/go/src/testing/testing.go:1851 +0x413
exit status 2
FAIL	github.com/sahilm/fuzzy	0.004s

@sahilm sahilm left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM. Thanks.

@sahilm sahilm merged commit 5ed613f into sahilm:master Aug 2, 2025
1 check passed
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