Master Sync

[mkacmaz]

Master Sync

Summary by CodeRabbit

• New Features

  • Added HTTP connection hijacking support for advanced protocol handling (e.g., WebSocket upgrades).

• Bug Fixes

  • Safer pattern parameter extraction to prevent runtime errors on edge cases.
  • Made context state access thread-safe to avoid race conditions during concurrent use.

• Chores

  • Improved context data propagation and cloning to ensure memory safety and consistent behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Context was made thread-safe by adding a RWMutex and safe accessors; header/cookie cloning replaced manual copies; response writer gained HTTP Hijack support; pattern matching now guards against invalid slice indices to avoid out-of-bounds behavior.

Changes

Cohort / File(s)	Summary
Thread-safe Context & accessors context.go	Added mu sync.RWMutex; made some fields immutable; introduced Set, Get, MustGet, Delete, and Params() with mutex protection; Deadline, Done, and Err use mutex; Done channel initialized on demand.
Context cloning on subcontext update context.go	Replaced manual header/cookie copying with maps.Clone(ctx.Headers) and slices.Clone(ctx.Cookies) in NewSubContextWithNode and used clones when updating parent in tryUpdateParent.
HTTP hijacker support context_response_writer.go	Added Hijack() (net.Conn, *bufio.ReadWriter, error) delegating to underlying ResponseWriter when supported; added compile-time check var _ http.Hijacker = &ContextResponseWriter{}.
Pattern matching bounds safety pattern.go	In Pattern.MatchInto, guard against negative start/end indices; assign empty string for invalid indices instead of slicing out-of-bounds.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibble at mutexes, gentle and bright,
Cloned headers tucked in snug at night,
Hijack doors open when streams take flight,
Patterns guarded, slices held tight,
Hops of safety — code feels light. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Master Sync' is vague and does not meaningfully convey the specific changes in the PR. While commit messages reference RWMutex thread-safety additions, the title is generic and could apply to many different types of changes.	Consider using a more descriptive title like 'Add thread-safety with RWMutex to Context' or 'Implement thread-safe context value management' to clearly communicate the primary change.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

context.go (2)

118-147: Guard parent reads and deep-clone headers/cookies to avoid shared mutable state.

NewSubContextWithNode reads ctx fields and copies Headers/Cookies without locking, and the current cloning is shallow (header value slices and cookie pointers are still shared). That can race or bleed mutations across contexts.

Consider taking an RLock around the snapshot and deep-cloning nested data.

🔒 Suggested fix (snapshot under lock + deep clone)

 func NewSubContextWithNode(ctx *Context, firstHandlerNode *HandlerNode) *Context {
 	subContext := contextFromPool()
 
+	ctx.mu.RLock()
+	defer ctx.mu.RUnlock()
+
 	subContext.parentContext = ctx
 
 	subContext.request = ctx.request
 
 	subContext.method = ctx.method
 	subContext.path = ctx.path
 	for k, v := range ctx.params {
 		subContext.params[k] = v
 	}
 
 	subContext.Status = ctx.Status
-	subContext.Headers = maps.Clone(ctx.Headers)
-	subContext.Cookies = slices.Clone(ctx.Cookies)
+	subContext.Headers = cloneHeader(ctx.Headers)
+	subContext.Cookies = cloneCookies(ctx.Cookies)
 	subContext.Body = ctx.Body
 	subContext.bodyWriter = ctx.bodyWriter
 	subContext.hasWrittenHeaders = ctx.hasWrittenHeaders
 	subContext.hasWrittenBody = ctx.hasWrittenBody
@@
 	for k, v := range ctx.associatedValues {
 		subContext.associatedValues[k] = v
 	}
 
 	subContext.deadline = ctx.deadline
 	subContext.doneChannel = ctx.doneChannel

Additional helpers (outside this range):

func cloneHeader(h http.Header) http.Header {
	if h == nil {
		return nil
	}
	dst := make(http.Header, len(h))
	for k, v := range h {
		vv := make([]string, len(v))
		copy(vv, v)
		dst[k] = vv
	}
	return dst
}

func cloneCookies(src []*http.Cookie) []*http.Cookie {
	if src == nil {
		return nil
	}
	dst := make([]*http.Cookie, len(src))
	for i, c := range src {
		if c != nil {
			cc := *c
			dst[i] = &cc
		}
	}
	return dst
}

---

223-245: Update parent under lock and avoid shallow copies.

tryUpdateParent mutates the parent without locking and uses shallow copies for headers/cookies. If the parent context is read concurrently, this is a data race and can leak mutations across contexts.

Snapshot c under RLock, then lock parentContext to apply the update, and reuse the deep-clone helpers.

🔒 Suggested fix (snapshot then apply under parent lock)

 func (c *Context) tryUpdateParent() {
 	if c.parentContext == nil {
 		return
 	}
+
+	c.mu.RLock()
+	status := c.Status
+	headers := cloneHeader(c.Headers)
+	cookies := cloneCookies(c.Cookies)
+	body := c.Body
+	hasWrittenHeaders := c.hasWrittenHeaders
+	hasWrittenBody := c.hasWrittenBody
+	maxBodySize := c.MaxRequestBodySize
+	err := c.Error
+	errStack := c.ErrorStack
+	finalErr := c.FinalError
+	finalErrStack := c.FinalErrorStack
+	reqUnmarshaller := c.requestBodyUnmarshaller
+	respMarshaller := c.responseBodyMarshaller
+	associated := make(map[string]any, len(c.associatedValues))
+	for k, v := range c.associatedValues {
+		associated[k] = v
+	}
+	deadline := c.deadline
+	doneCh := c.doneChannel
+	c.mu.RUnlock()
 
-	c.parentContext.Status = c.Status
-	c.parentContext.Headers = maps.Clone(c.Headers)
-	c.parentContext.Cookies = slices.Clone(c.Cookies)
-	c.parentContext.Body = c.Body
-	c.parentContext.hasWrittenHeaders = c.hasWrittenHeaders
-	c.parentContext.hasWrittenBody = c.hasWrittenBody
+	p := c.parentContext
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	p.Status = status
+	p.Headers = headers
+	p.Cookies = cookies
+	p.Body = body
+	p.hasWrittenHeaders = hasWrittenHeaders
+	p.hasWrittenBody = hasWrittenBody
 
-	c.parentContext.MaxRequestBodySize = c.MaxRequestBodySize
+	p.MaxRequestBodySize = maxBodySize
 
-	c.parentContext.Error = c.Error
-	c.parentContext.ErrorStack = c.ErrorStack
-	c.parentContext.FinalError = c.FinalError
-	c.parentContext.FinalErrorStack = c.FinalErrorStack
+	p.Error = err
+	p.ErrorStack = errStack
+	p.FinalError = finalErr
+	p.FinalErrorStack = finalErrStack
 
-	c.parentContext.requestBodyUnmarshaller = c.requestBodyUnmarshaller
-	c.parentContext.responseBodyMarshaller = c.responseBodyMarshaller
+	p.requestBodyUnmarshaller = reqUnmarshaller
+	p.responseBodyMarshaller = respMarshaller
 
-	for k, v := range c.associatedValues {
-		c.parentContext.associatedValues[k] = v
-	}
-	c.parentContext.deadline = c.deadline
-	c.parentContext.doneChannel = c.doneChannel
+	for k, v := range associated {
+		p.associatedValues[k] = v
+	}
+	p.deadline = deadline
+	p.doneChannel = doneCh
 }

🤖 Fix all issues with AI agents

In `@context.go`:
- Around line 268-275: The MustGet method on Context currently returns nil when
a key is absent but should panic per its contract; update Context.MustGet to
check map membership after acquiring the read lock (use the comma-ok pattern on
c.associatedValues[key]) and if the key is missing call panic with a clear
message (e.g., include the missing key and Context type), otherwise return the
value; ensure you still use c.mu.RLock()/RUnlock() around the lookup and only
panic when the key is not found.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

MustGet doesn’t honor its panic-on-miss contract.

The doc says it panics if the key is missing, but the current implementation returns nil instead.

✅ Suggested fix

 func (c *Context) MustGet(key string) any {
 	c.mu.RLock()
-	v := c.associatedValues[key]
+	v, ok := c.associatedValues[key]
 	c.mu.RUnlock()
+	if !ok {
+		panic("context value not found: " + key)
+	}
 	return v
 }

🤖 Prompt for AI Agents

In `@context.go` around lines 268 - 275, The MustGet method on Context currently
returns nil when a key is absent but should panic per its contract; update
Context.MustGet to check map membership after acquiring the read lock (use the
comma-ok pattern on c.associatedValues[key]) and if the key is missing call
panic with a clear message (e.g., include the missing key and Context type),
otherwise return the value; ensure you still use c.mu.RLock()/RUnlock() around
the lookup and only panic when the key is not found.