Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/gclient: chaining function Cookie change the original client cookie #3901

Closed
CharLemAznable opened this issue Oct 30, 2024 · 0 comments
Closed
Labels
bug It is confirmed a bug, but don't worry, we'll handle it. done This issue is done, which may be release in next version.

Comments

@CharLemAznable
Copy link
Contributor

CharLemAznable commented Oct 30, 2024

Go version

go version go1.20.4 darwin/amd64

GoFrame version

2.7.4

Can this bug be reproduced with the latest release?

Option Yes

What did you do?

When I using gclient.Client chaining function Cookie, I found an unexpected error.

func Test_Client_Chain_Cookie(t *testing.T) {
	s := g.Server(guid.S())
	s.BindHandler("/cookie", func(r *ghttp.Request) {
		r.Response.Write(r.Cookie.Get("test", "def")) // response cookie's value, or `def` if cookie does not exist
	})
	s.SetDumpRouterMap(false)
	s.Start()
	defer s.Shutdown()

	ctx := context.Background()
	time.Sleep(100 * time.Millisecond)
	gtest.C(t, func(t *gtest.T) {
		c := g.Client()
		c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

		// client c does not have cookie, return `def`
		t.Assert(c.GetContent(ctx, "/cookie"), "def")

		// chaining function returns client c's copy with cookie
		copyWithCookie := c.Cookie(g.MapStrStr{"test": "1234567890"})
		t.Assert(copyWithCookie.GetContent(ctx, "/cookie"), "1234567890")

		// client c didn't change, does not have cookie still, should return `def`
		t.Assert(c.GetContent(ctx, "/cookie"), "def")
	})
}

What did you see happen?

But test result like:

[ASSERT] EXPECT 1234567890 == def

What did you expect to see?

I found the Clone function of gclient.Client:

func (c *Client) Clone() *Client {
	newClient := New()
	*newClient = *c
	if len(c.header) > 0 {
		newClient.header = make(map[string]string)
		for k, v := range c.header {
			newClient.header[k] = v
		}
	}
	if len(c.cookies) > 0 {
		newClient.cookies = make(map[string]string)
		for k, v := range c.cookies {
			newClient.cookies[k] = v
		}
	}
	return newClient
}

So, if the original client has no header or cookie, then the new client would not allocate header or cookie field.

And, in New function, a new client always create with User-Agent header, so this problem didn't happened with chaining function Header.

Maybe I can fix this problem by submit a pull request like this?

func (c *Client) Clone() *Client {
	newClient := New()
	*newClient = *c
	newClient.header = make(map[string]string)
	if len(c.header) > 0 {
		for k, v := range c.header {
			newClient.header[k] = v
		}
	}
	newClient.cookies = make(map[string]string)
	if len(c.cookies) > 0 {
		for k, v := range c.cookies {
			newClient.cookies[k] = v
		}
	}
	return newClient
}
@CharLemAznable CharLemAznable added the bug It is confirmed a bug, but don't worry, we'll handle it. label Oct 30, 2024
@gqcn gqcn added the done This issue is done, which may be release in next version. label Nov 12, 2024
@gqcn gqcn closed this as completed Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It is confirmed a bug, but don't worry, we'll handle it. done This issue is done, which may be release in next version.
Projects
None yet
Development

No branches or pull requests

2 participants