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

SetRequestCallbacks not working #241

Closed
joseph-reslv opened this issue Oct 13, 2023 · 1 comment · Fixed by #242
Closed

SetRequestCallbacks not working #241

joseph-reslv opened this issue Oct 13, 2023 · 1 comment · Fixed by #242

Comments

@joseph-reslv
Copy link

Expected Behavior

When using SetRequestCallbacks, i expected the callbacks should be set to the requestCallbacks in client.

client.SetRequestCallbacks(func(req *http.Request) {
    fmt.Println("client callback")
})

Current Behavior

The requestCallbacks in client is still empty.

I think the reason the requestCallbacks not being copy is due to the copy function.

func (c *Client) SetRequestCallbacks(callbacks ...RequestCallback) error {

// SetRequestCallbacks sets callbacks which will be invoked before each request.
func (c *Client) SetRequestCallbacks(callbacks ...RequestCallback) error {
	c.clientRequestModifiersLock.Lock()
	copy(c.clientRequestModifiers.requestCallbacks, callbacks)
	c.clientRequestModifiersLock.Unlock()

	return nil
}

As mentioned in the Copy,

Copy returns the number of elements copied, which will be the minimum of len(src) and len(dst).

So, when the client is initialized as no requestCallbacks, the copy function will always copy nothing to the requestCallbacks.

To solve the bug, I think you can change to use append to fix it.

c.clientRequestModifiers.requestCallbacks = append(c.clientRequestModifiers.requestCallbacks, callbacks...)

Failure Information

vault-client-go: 0.4.1

Please include the version of Vault binary and the version of vault-client-go you're using.

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

client, _ := vault.New()

client.SetRequestCallbacks(func(req *http.Request) {
    fmt.Println("client callback")
})

Additional Information

I think the SetResponseCallbacks having same issue.

func (c *Client) SetResponseCallbacks(callbacks ...ResponseCallback) error {

@averche
Copy link
Collaborator

averche commented Oct 18, 2023

Thanks for reporting this, @joseph-reslv! This is indeed an issue that somehow got overlooked, I'm fixing it in #242.

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

Successfully merging a pull request may close this issue.

2 participants