Skip to content

Conversation

jackcipher
Copy link

@jackcipher jackcipher commented Jun 16, 2025

Problem

There is a critical bug in the cookie cloning logic of Client.Clone() . The current implementation:

cc.cookies = make([]*http.Cookie, l)
for _, cookie := range c.cookies {
    cc.cookies = append(cc.cookies, cloneCookie(cookie))
}

This creates two issues:

  1. The initial make([]*http.Cookie, l) creates a slice with l nil elements
  2. append then adds the actual cookies after these nil elements

Result: A slice with 2l length where:

  • First l elements are nil
  • Last l elements are the actual cookies

image

Panic Example

The following code will trigger a panic:

resty.New().SetCookie(&http.Cookie{
  Name:  "k",
  Value: "v",
 }).Clone(context.Background()).R().Get("about:blank")

This happens because operations like AddCookie may attempt to access the nil cookies, leading to a panic.

Changes Made

  • Fixed cookie cloning in Client.Clone() method
  • Added test cases to verify cookie cloning behavior

Test Coverage

Added new test assertions in TestClientClone to verify:

  • Cookie length equality between parent and clone
  • Cookie name and value equality for each cookie

Impact

This fix ensures that when a client is cloned, all cookies are properly copied to the new instance, maintaining the exact same cookie state as the parent client.

Testing

All existing tests pass, and new test cases have been added to verify the fix.

@jackcipher
Copy link
Author

@jeevatkm Hi! Just wanted to check in on this bug fix. Is there anything I can do to help move this forward? Happy to address any feedback or make changes if needed. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant