Fix potential data race and TOCTOU inconsistency in route operation

[somak2kai]

What?

Fixes issue : #6148

The fix addresses a potential data race in https://github.com/grafana/k6/blob/master/internal/js/modules/k6/browser/common/page.go#L1356

Why?

in internal/js/modules/k6/browser/common/page.go: Route method had an updateRequestInterception as true in its method body, however this was not protected under any locks. if more than 1 goroutine fires for the same api and calls route, there is an obvious data race on thestate update of updateRequestInterception. Additionally if a route and unroute is called by different goroutines , the interception mode on the api would be in an inconsistent state. it may be turned on or off.

Affected code links : https://github.com/grafana/k6/blob/master/internal/js/modules/k6/browser/common/page.go#L1356

There is a steps to reproduce unit test also provided in this pr as well as in the issue link #6148

Checklist

• I have performed a self-review of my code.
• I have commented on my code, particularly in hard-to-understand areas.
• I have added tests for my changes.
• I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

• I have added the correct milestone and labels to the PR.
• I have updated the release notes: link
• I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
• I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

#6148

[Copilot]

Pull request overview

This PR addresses a concurrency issue in the k6 browser module’s route registration by making route enablement and request-interception toggling atomic with respect to the page’s route list, and adds a regression test aimed at surfacing the original race under the Go race detector.

Changes:

• Serialize Page.Route()’s first-route registration logic with routesMu to prevent concurrent “first route” enablement races.
• Add a concurrency test that performs parallel route registrations to catch the race with go test -race.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
internal/js/modules/k6/browser/common/page.go	Moves route-list locking to cover the “first route enables interception” check, preventing concurrent enablement races.
internal/js/modules/k6/browser/tests/page_test.go	Adds a parallel/goroutine-based test intended to reproduce the original race during concurrent Page.Route() calls.

Comments suppressed due to low confidence (1)

internal/js/modules/k6/browser/common/page.go:1365

• Request interception is enabled before validating that a route can actually be registered (i.e., before newPatternMatcher succeeds). If matcher creation fails (e.g., rm is nil), this leaves request interception enabled with zero routes, which is an observable side effect (fetch interception + cache disabled) despite returning an error. Consider creating the matcher/RouteHandler first, then taking routesMu and enabling interception only when you’re sure you’ll append the route.

	if len(p.routes) == 0 {
		err := p.mainFrameSession.updateRequestInterception(true)
		if err != nil {
			return err
		}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[szkiba]

@somak2kai thank you for your contribution!

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

internal/js/modules/k6/browser/common/page.go:1366

• Route() enables request interception before validating/constructing the route matcher/handler. If newPatternMatcher() returns an error (e.g., nil RegExMatcher) the function returns with routes still empty but interception already enabled, leaving the page/network manager in an inconsistent state. You can avoid this and also reduce time holding routesMu by building the matcher/handler first, then taking the lock and toggling interception only when you’re sure a route will be registered.

	p.routesMu.Lock()
	defer p.routesMu.Unlock()
	if len(p.routes) == 0 {
		err := p.mainFrameSession.updateRequestInterception(true)
		if err != nil {
			return err
		}
	}

[joanlopez]

LGTM, thanks for the contribution @somak2kai! 🫶