fix: panic in linear myers algorithm

[smira]

In linearSpaceMyersRecWithIndices, the suffix-scanning loop:

for suffix < n-prefix && a[aEnd-1-suffix] == b[bEnd-1-suffix] {

bounds the scan against n-prefix (a's remaining length after prefix) but not against m-prefix (b's remaining length).

With the fuzzer's input a="\n\n\n\n\n\n\n\n\n0" → 10 lines, b="0" → 1 line:

• n=10, m=1, prefix=0
• suffix=0: a[9]="0" == b[0]="0" → true, suffix becomes 1 suffix=1: tries b[1-1-1] = b[-1] → index out of range panic

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug in the linearSpaceMyersRecWithIndices function of the Myers diff algorithm. The fix prevents an index out-of-range panic that could occur during suffix scanning when comparing sequences of specific lengths, ensuring the algorithm correctly bounds its access to both input slices.

Highlights

• Panic Fix: Resolved an index out-of-range panic in the linearSpaceMyersRecWithIndices function by adding a missing boundary check for the b slice during the suffix-scanning loop.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request addresses a panic in the Myers diff algorithm's suffix-scanning loop by adding a necessary bounds check. While the fix for the suffix loop is correct, I've identified an identical vulnerability in the prefix-scanning loop just a few lines above the change. This related bug can also cause a panic. I've left a critical review comment on the changed line, detailing the issue and providing a code snippet to fix the vulnerable prefix loop.

[smira]

I ended up fixing more panics, all found via fuzz testing with algorithm locked to linear Myers.

[KimNorgaard]

@smira Thanks for the PR. Nice find!

I wrote two tests that confirm the clamping and suffix limit panics:

func TestLinearSpacePanics(t *testing.T) {
	// x/y clamping panic
	t.Run("xy clamping panic", func(t *testing.T) {
		a := []string{"A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K"}
		b := []string{"X", "Y", "A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "Z"}

		_, err := myers.DiffStrings(a, b,
			myers.WithLinearSpace(true),
			myers.WithSmallInputThreshold(0),
		)
		assert.NoError(t, err)
	})

	// prefix/suffix limit panic
	t.Run("prefix suffix limit panic", func(t *testing.T) {
		a := []string{"X", "X", "X", "X", "X", "X", "X", "X", "X", "X", "X"}
		b := []string{"X", "X", "X", "X", "X", "X", "X", "X", "X", "X"}

		_, err := myers.DiffStrings(a, b,
			myers.WithLinearSpace(true),
			myers.WithSmallInputThreshold(0),
		)
		assert.NoError(t, err)
	})
}

I'll add the tests after merging the PR.

[KimNorgaard]

#45
#46
#47

[KimNorgaard]

@smira just wanted to give you a heads up that i pushed v1.0.1

edited: tagged v0.2.2 but realized we had published and retracted v1.0.0 some time ago and go really wants to use the newest version even though it's retracted