From c0e50a6b466271f914605b4a88f0397cf2152c5f Mon Sep 17 00:00:00 2001 From: Norwin Roosen Date: Wed, 3 Mar 2021 07:55:41 +0100 Subject: [PATCH 1/6] docs: mention thread-safeness --- gitea/client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gitea/client.go b/gitea/client.go index 6e579e8..0767a6d 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -26,7 +26,7 @@ func Version() string { return "0.14.0" } -// Client represents a Gitea API client. +// Client represents a thread-safe Gitea API client. type Client struct { url string accessToken string @@ -47,6 +47,7 @@ type Response struct { } // NewClient initializes and returns a API client. +// Usage of all gitea.Client methods is concurrency-safe. func NewClient(url string, options ...func(*Client)) (*Client, error) { client := &Client{ url: strings.TrimSuffix(url, "/"), -- 2.40.1 From 7084a556a970917f0b595e6807446ba15b3de89b Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 4 Mar 2021 00:03:34 +0100 Subject: [PATCH 2/6] use mutex --- gitea/client.go | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/gitea/client.go b/gitea/client.go index 0767a6d..01042f0 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -37,6 +37,7 @@ type Client struct { debug bool client *http.Client ctx context.Context + mutex sync.RWMutex serverVersion *version.Version getVersionOnce sync.Once } @@ -73,14 +74,23 @@ func NewClientWithHTTP(url string, httpClient *http.Client) *Client { // SetHTTPClient is an option for NewClient to set custom http client func SetHTTPClient(httpClient *http.Client) func(client *Client) { return func(client *Client) { - client.client = httpClient + client.SetHTTPClient(httpClient) } } +// SetHTTPClient replaces default http.Client with user given one. +func (c *Client) SetHTTPClient(client *http.Client) { + c.mutex.Lock() + c.client = client + c.mutex.Unlock() +} + // SetToken is an option for NewClient to set token func SetToken(token string) func(client *Client) { return func(client *Client) { + client.mutex.Lock() client.accessToken = token + client.mutex.Unlock() } } @@ -93,7 +103,9 @@ func SetBasicAuth(username, password string) func(client *Client) { // SetBasicAuth sets username and password func (c *Client) SetBasicAuth(username, password string) { + c.mutex.Lock() c.username, c.password = username, password + c.mutex.Unlock() } // SetOTP is an option for NewClient to set OTP for 2FA @@ -105,7 +117,9 @@ func SetOTP(otp string) func(client *Client) { // SetOTP sets OTP for 2FA func (c *Client) SetOTP(otp string) { + c.mutex.Lock() c.otp = otp + c.mutex.Unlock() } // SetContext is an option for NewClient to set context @@ -117,12 +131,9 @@ func SetContext(ctx context.Context) func(client *Client) { // SetContext set context witch is used for http requests func (c *Client) SetContext(ctx context.Context) { + c.mutex.Lock() c.ctx = ctx -} - -// SetHTTPClient replaces default http.Client with user given one. -func (c *Client) SetHTTPClient(client *http.Client) { - c.client = client + c.mutex.Unlock() } // SetSudo is an option for NewClient to set sudo header @@ -134,7 +145,9 @@ func SetSudo(sudo string) func(client *Client) { // SetSudo sets username to impersonate. func (c *Client) SetSudo(sudo string) { + c.mutex.Lock() c.sudo = sudo + c.mutex.Unlock() } // SetDebugMode is an option for NewClient to enable debug mode @@ -145,13 +158,18 @@ func SetDebugMode() func(client *Client) { } func (c *Client) getWebResponse(method, path string, body io.Reader) ([]byte, *Response, error) { + c.mutex.RLock() if c.debug { fmt.Printf("%s: %s\nBody: %v\n", method, c.url+path, body) } req, err := http.NewRequestWithContext(c.ctx, method, c.url+path, body) if err != nil { + c.mutex.RUnlock() return nil, nil, err } + + c.mutex.RUnlock() + resp, err := c.client.Do(req) if err != nil { return nil, nil, err @@ -166,11 +184,13 @@ func (c *Client) getWebResponse(method, path string, body io.Reader) ([]byte, *R } func (c *Client) doRequest(method, path string, header http.Header, body io.Reader) (*Response, error) { + c.mutex.RLock() if c.debug { fmt.Printf("%s: %s\nHeader: %v\nBody: %s\n", method, c.url+"/api/v1"+path, header, body) } req, err := http.NewRequestWithContext(c.ctx, method, c.url+"/api/v1"+path, body) if err != nil { + c.mutex.RUnlock() return nil, err } if len(c.accessToken) != 0 { @@ -185,6 +205,9 @@ func (c *Client) doRequest(method, path string, header http.Header, body io.Read if len(c.sudo) != 0 { req.Header.Set("Sudo", c.sudo) } + + c.mutex.RUnlock() + for k, v := range header { req.Header[k] = v } -- 2.40.1 From 969df574f9172810331ad4e2693cf26f66aa8954 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 4 Mar 2021 01:14:57 +0100 Subject: [PATCH 3/6] simplify & finish --- gitea/client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gitea/client.go b/gitea/client.go index 01042f0..9d77239 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -153,7 +153,9 @@ func (c *Client) SetSudo(sudo string) { // SetDebugMode is an option for NewClient to enable debug mode func SetDebugMode() func(client *Client) { return func(client *Client) { + client.mutex.Lock() client.debug = true + client.mutex.Unlock() } } @@ -163,13 +165,11 @@ func (c *Client) getWebResponse(method, path string, body io.Reader) ([]byte, *R fmt.Printf("%s: %s\nBody: %v\n", method, c.url+path, body) } req, err := http.NewRequestWithContext(c.ctx, method, c.url+path, body) + c.mutex.RUnlock() if err != nil { - c.mutex.RUnlock() return nil, nil, err } - c.mutex.RUnlock() - resp, err := c.client.Do(req) if err != nil { return nil, nil, err -- 2.40.1 From 6e7f81a3a62907e6488fcaba503a50a0f1628fac Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 4 Mar 2021 13:53:41 +0100 Subject: [PATCH 4/6] get client ref within thread-save code --- gitea/client.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gitea/client.go b/gitea/client.go index 9d77239..7bae185 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -165,12 +165,15 @@ func (c *Client) getWebResponse(method, path string, body io.Reader) ([]byte, *R fmt.Printf("%s: %s\nBody: %v\n", method, c.url+path, body) } req, err := http.NewRequestWithContext(c.ctx, method, c.url+path, body) + + client := c.client // client ref can change from this point on so safe it c.mutex.RUnlock() + if err != nil { return nil, nil, err } - resp, err := c.client.Do(req) + resp, err := client.Do(req) if err != nil { return nil, nil, err } @@ -206,13 +209,14 @@ func (c *Client) doRequest(method, path string, header http.Header, body io.Read req.Header.Set("Sudo", c.sudo) } + client := c.client // client ref can change from this point on so safe it c.mutex.RUnlock() for k, v := range header { req.Header[k] = v } - resp, err := c.client.Do(req) + resp, err := client.Do(req) if err != nil { return nil, err } -- 2.40.1 From 308a9e73caa759842ebd4192e5594d858b74711a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 4 Mar 2021 19:15:18 +0100 Subject: [PATCH 5/6] same with c.debug --- gitea/client.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/gitea/client.go b/gitea/client.go index 7bae185..1b22711 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -161,7 +161,8 @@ func SetDebugMode() func(client *Client) { func (c *Client) getWebResponse(method, path string, body io.Reader) ([]byte, *Response, error) { c.mutex.RLock() - if c.debug { + debug := c.debug + if debug { fmt.Printf("%s: %s\nBody: %v\n", method, c.url+path, body) } req, err := http.NewRequestWithContext(c.ctx, method, c.url+path, body) @@ -180,7 +181,7 @@ func (c *Client) getWebResponse(method, path string, body io.Reader) ([]byte, *R defer resp.Body.Close() data, err := ioutil.ReadAll(resp.Body) - if c.debug { + if debug { fmt.Printf("Response: %v\n\n", resp) } return data, &Response{resp}, nil @@ -188,7 +189,8 @@ func (c *Client) getWebResponse(method, path string, body io.Reader) ([]byte, *R func (c *Client) doRequest(method, path string, header http.Header, body io.Reader) (*Response, error) { c.mutex.RLock() - if c.debug { + debug := c.debug + if debug { fmt.Printf("%s: %s\nHeader: %v\nBody: %s\n", method, c.url+"/api/v1"+path, header, body) } req, err := http.NewRequestWithContext(c.ctx, method, c.url+"/api/v1"+path, body) @@ -220,7 +222,7 @@ func (c *Client) doRequest(method, path string, header http.Header, body io.Read if err != nil { return nil, err } - if c.debug { + if debug { fmt.Printf("Response: %v\n\n", resp) } return &Response{resp}, nil -- 2.40.1 From 1b45e79cf64cc4bcb66b516c418420f17f55ce2f Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 4 Mar 2021 19:24:19 +0100 Subject: [PATCH 6/6] find posible race cond outside client.go --- gitea/issue.go | 2 ++ gitea/user_app.go | 21 +++++++++++++++------ gitea/version.go | 10 ++++++++-- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/gitea/issue.go b/gitea/issue.go index a5280e7..211cb24 100644 --- a/gitea/issue.go +++ b/gitea/issue.go @@ -253,6 +253,8 @@ func (c *Client) EditIssue(owner, repo string, index int64, opt EditIssueOption) func (c *Client) issueBackwardsCompatibility(issue *Issue) { if c.checkServerVersionGreaterThanOrEqual(version1_12_0) != nil { + c.mutex.RLock() issue.HTMLURL = fmt.Sprintf("%s/%s/issues/%d", c.url, issue.Repository.FullName, issue.Index) + c.mutex.RUnlock() } } diff --git a/gitea/user_app.go b/gitea/user_app.go index 7f7696d..cf6c3cf 100644 --- a/gitea/user_app.go +++ b/gitea/user_app.go @@ -27,12 +27,15 @@ type ListAccessTokensOptions struct { // ListAccessTokens lists all the access tokens of user func (c *Client) ListAccessTokens(opts ListAccessTokensOptions) ([]*AccessToken, *Response, error) { - if len(c.username) == 0 { + c.mutex.RLock() + username := c.username + c.mutex.RUnlock() + if len(username) == 0 { return nil, nil, fmt.Errorf("\"username\" not set: only BasicAuth allowed") } opts.setDefaults() tokens := make([]*AccessToken, 0, opts.PageSize) - resp, err := c.getParsedResponse("GET", fmt.Sprintf("/users/%s/tokens?%s", c.username, opts.getURLQuery().Encode()), jsonHeader, nil, &tokens) + resp, err := c.getParsedResponse("GET", fmt.Sprintf("/users/%s/tokens?%s", username, opts.getURLQuery().Encode()), jsonHeader, nil, &tokens) return tokens, resp, err } @@ -43,7 +46,10 @@ type CreateAccessTokenOption struct { // CreateAccessToken create one access token with options func (c *Client) CreateAccessToken(opt CreateAccessTokenOption) (*AccessToken, *Response, error) { - if len(c.username) == 0 { + c.mutex.RLock() + username := c.username + c.mutex.RUnlock() + if len(username) == 0 { return nil, nil, fmt.Errorf("\"username\" not set: only BasicAuth allowed") } body, err := json.Marshal(&opt) @@ -51,13 +57,16 @@ func (c *Client) CreateAccessToken(opt CreateAccessTokenOption) (*AccessToken, * return nil, nil, err } t := new(AccessToken) - resp, err := c.getParsedResponse("POST", fmt.Sprintf("/users/%s/tokens", c.username), jsonHeader, bytes.NewReader(body), t) + resp, err := c.getParsedResponse("POST", fmt.Sprintf("/users/%s/tokens", username), jsonHeader, bytes.NewReader(body), t) return t, resp, err } // DeleteAccessToken delete token, identified by ID and if not available by name func (c *Client) DeleteAccessToken(value interface{}) (*Response, error) { - if len(c.username) == 0 { + c.mutex.RLock() + username := c.username + c.mutex.RUnlock() + if len(username) == 0 { return nil, fmt.Errorf("\"username\" not set: only BasicAuth allowed") } @@ -75,6 +84,6 @@ func (c *Client) DeleteAccessToken(value interface{}) (*Response, error) { return nil, fmt.Errorf("only string and int64 supported") } - _, resp, err := c.getResponse("DELETE", fmt.Sprintf("/users/%s/tokens/%s", c.username, token), jsonHeader, nil) + _, resp, err := c.getResponse("DELETE", fmt.Sprintf("/users/%s/tokens/%s", username, token), jsonHeader, nil) return resp, err } diff --git a/gitea/version.go b/gitea/version.go index c96ef66..ed8a2ae 100644 --- a/gitea/version.go +++ b/gitea/version.go @@ -31,7 +31,10 @@ func (c *Client) CheckServerVersionConstraint(constraint string) error { return err } if !check.Check(c.serverVersion) { - return fmt.Errorf("gitea server at %s does not satisfy version constraint %s", c.url, constraint) + c.mutex.RLock() + url := c.url + c.mutex.RUnlock() + return fmt.Errorf("gitea server at %s does not satisfy version constraint %s", url, constraint) } return nil } @@ -51,7 +54,10 @@ func (c *Client) checkServerVersionGreaterThanOrEqual(v *version.Version) error } if !c.serverVersion.GreaterThanOrEqual(v) { - return fmt.Errorf("gitea server at %s is older than %s", c.url, v.Original()) + c.mutex.RLock() + url := c.url + c.mutex.RUnlock() + return fmt.Errorf("gitea server at %s is older than %s", url, v.Original()) } return nil } -- 2.40.1