From f14c63635ccc20854aa6bcd86e0c8140f3b8d7a3 Mon Sep 17 00:00:00 2001 From: xgopilot Date: Thu, 6 Nov 2025 06:32:31 +0000 Subject: [PATCH 1/2] fix: resolve thread-safety issues in uptoken package Fixes #169 This commit addresses data races in both signer and parser implementations: 1. signer.onceGetUpToken: Added sync.Once to protect concurrent upToken generation - Added onceUpToken sync.Once field - Added upTokenErr field to store initialization errors - Ensures token is generated exactly once across all goroutines 2. parser.GetPutPolicy: Added sync.Once to protect json.Unmarshal - Added oncePutPolicy sync.Once field - Added putPolicyErr field to store parsing errors - Prevents concurrent access to putPolicy field 3. parser.GetAccessKey: Added sync.Once for thread-safe access - Added onceAccessKey sync.Once field - Added accessKeyErr field to store errors - Protects accessKey field from concurrent writes 4. parser.onceGetSplits: Added sync.Once for thread-safe parsing - Added onceSplits sync.Once field - Added splitsValid field to track parsing result - Ensures splits are parsed exactly once All lazy initialization patterns now use sync.Once which guarantees: - Thread-safe initialization - Exactly-once execution - Proper memory barriers Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: zhangzqs <34616640+zhangzqs@users.noreply.github.com> --- storagev2/uptoken/uploadtoken.go | 115 +++++++++++++++++-------------- 1 file changed, 63 insertions(+), 52 deletions(-) diff --git a/storagev2/uptoken/uploadtoken.go b/storagev2/uptoken/uploadtoken.go index b3c32685..265ecddf 100644 --- a/storagev2/uptoken/uploadtoken.go +++ b/storagev2/uptoken/uploadtoken.go @@ -37,14 +37,22 @@ type ( putPolicy PutPolicy credentialsProvider credentials.CredentialsProvider onceCredentials sync.Once + onceUpToken sync.Once upToken string + upTokenErr error credentials *credentials.Credentials } parser struct { - upToken string - putPolicy PutPolicy - accessKey string - splits []string + upToken string + oncePutPolicy sync.Once + putPolicy PutPolicy + putPolicyErr error + onceAccessKey sync.Once + accessKey string + accessKeyErr error + onceSplits sync.Once + splits []string + splitsValid bool } ) @@ -85,20 +93,20 @@ func (signer *signer) onceGetCredentials(ctx context.Context) (*credentials.Cred } func (signer *signer) onceGetUpToken(ctx context.Context) (string, error) { - var err error - if signer.upToken != "" { - return signer.upToken, nil - } - credentials, err := signer.onceGetCredentials(ctx) - if err != nil { - return "", nil - } - putPolicyJson, err := json.Marshal(signer.putPolicy) - if err != nil { - return "", nil - } - signer.upToken = credentials.SignWithData(putPolicyJson) - return signer.upToken, nil + signer.onceUpToken.Do(func() { + var credentials *credentials.Credentials + credentials, signer.upTokenErr = signer.onceGetCredentials(ctx) + if signer.upTokenErr != nil { + return + } + var putPolicyJson []byte + putPolicyJson, signer.upTokenErr = json.Marshal(signer.putPolicy) + if signer.upTokenErr != nil { + return + } + signer.upToken = credentials.SignWithData(putPolicyJson) + }) + return signer.upToken, signer.upTokenErr } // NewParser 创建上传凭证签发器 @@ -107,46 +115,49 @@ func NewParser(upToken string) Provider { } func (parser *parser) GetPutPolicy(context.Context) (PutPolicy, error) { - if parser.putPolicy != nil { - return parser.putPolicy, nil - } - splits, ok := parser.onceGetSplits() - if !ok { - return nil, ErrInvalidUpToken - } - putPolicyJson, err := base64.URLEncoding.DecodeString(splits[2]) - if err != nil { - return nil, ErrInvalidUpToken - } - err = json.Unmarshal(putPolicyJson, &parser.putPolicy) - return parser.putPolicy, err + parser.oncePutPolicy.Do(func() { + splits, ok := parser.onceGetSplits() + if !ok { + parser.putPolicyErr = ErrInvalidUpToken + return + } + var putPolicyJson []byte + putPolicyJson, parser.putPolicyErr = base64.URLEncoding.DecodeString(splits[2]) + if parser.putPolicyErr != nil { + parser.putPolicyErr = ErrInvalidUpToken + return + } + parser.putPolicyErr = json.Unmarshal(putPolicyJson, &parser.putPolicy) + }) + return parser.putPolicy, parser.putPolicyErr } func (parser *parser) GetAccessKey(context.Context) (string, error) { - if parser.accessKey != "" { - return parser.accessKey, nil - } - splits, ok := parser.onceGetSplits() - if !ok { - return "", ErrInvalidUpToken - } - parser.accessKey = splits[0] - return parser.accessKey, nil + parser.onceAccessKey.Do(func() { + splits, ok := parser.onceGetSplits() + if !ok { + parser.accessKeyErr = ErrInvalidUpToken + return + } + parser.accessKey = splits[0] + }) + return parser.accessKey, parser.accessKeyErr } func (parser *parser) onceGetSplits() ([]string, bool) { - if len(parser.splits) > 0 { - return parser.splits, true - } - splits := strings.Split(parser.upToken, ":") - if len(splits) == 5 && splits[0] == "" { - splits = splits[2:] - } - if len(splits) != 3 { - return nil, false - } - parser.splits = splits - return parser.splits, true + parser.onceSplits.Do(func() { + splits := strings.Split(parser.upToken, ":") + if len(splits) == 5 && splits[0] == "" { + splits = splits[2:] + } + if len(splits) != 3 { + parser.splitsValid = false + return + } + parser.splits = splits + parser.splitsValid = true + }) + return parser.splits, parser.splitsValid } func (parser *parser) GetUpToken(context.Context) (string, error) { From 379c270ec3a80324d6b060c2fc4adf7ec0339990 Mon Sep 17 00:00:00 2001 From: xgopilot Date: Thu, 6 Nov 2025 06:38:18 +0000 Subject: [PATCH 2/2] refactor: remove error fields from structs, use mutex instead of sync.Once Replace sync.Once + error fields pattern with sync.Mutex + initialization flags. This removes all xxxErr fields (upTokenErr, putPolicyErr, accessKeyErr) from the structs while maintaining thread-safety through mutex locks. Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: zhangzqs <34616640+zhangzqs@users.noreply.github.com> --- storagev2/uptoken/uploadtoken.go | 216 ++++++++++++++++++++----------- 1 file changed, 140 insertions(+), 76 deletions(-) diff --git a/storagev2/uptoken/uploadtoken.go b/storagev2/uptoken/uploadtoken.go index 265ecddf..a4c66df9 100644 --- a/storagev2/uptoken/uploadtoken.go +++ b/storagev2/uptoken/uploadtoken.go @@ -36,23 +36,21 @@ type ( signer struct { putPolicy PutPolicy credentialsProvider credentials.CredentialsProvider - onceCredentials sync.Once - onceUpToken sync.Once - upToken string - upTokenErr error + mu sync.Mutex credentials *credentials.Credentials + credentialsInited bool + upToken string + upTokenInited bool } parser struct { - upToken string - oncePutPolicy sync.Once - putPolicy PutPolicy - putPolicyErr error - onceAccessKey sync.Once - accessKey string - accessKeyErr error - onceSplits sync.Once - splits []string - splitsValid bool + upToken string + mu sync.Mutex + putPolicy PutPolicy + putPolicyInited bool + accessKey string + accessKeyInited bool + splits []string + splitsInited bool } ) @@ -68,8 +66,7 @@ func (signer *signer) GetPutPolicy(context.Context) (PutPolicy, error) { } func (signer *signer) GetAccessKey(ctx context.Context) (string, error) { - var err error - credentials, err := signer.onceGetCredentials(ctx) + credentials, err := signer.getCredentials(ctx) if err != nil { return "", err } @@ -77,36 +74,60 @@ func (signer *signer) GetAccessKey(ctx context.Context) (string, error) { } func (signer *signer) GetUpToken(ctx context.Context) (string, error) { - return signer.onceGetUpToken(ctx) + signer.mu.Lock() + if signer.upTokenInited { + defer signer.mu.Unlock() + return signer.upToken, nil + } + signer.mu.Unlock() + + credentials, err := signer.getCredentials(ctx) + if err != nil { + return "", err + } + + putPolicyJson, err := json.Marshal(signer.putPolicy) + if err != nil { + return "", err + } + + upToken := credentials.SignWithData(putPolicyJson) + + signer.mu.Lock() + defer signer.mu.Unlock() + if !signer.upTokenInited { + signer.upToken = upToken + signer.upTokenInited = true + } + return signer.upToken, nil } -func (signer *signer) onceGetCredentials(ctx context.Context) (*credentials.Credentials, error) { +func (signer *signer) getCredentials(ctx context.Context) (*credentials.Credentials, error) { + signer.mu.Lock() + if signer.credentialsInited { + defer signer.mu.Unlock() + return signer.credentials, nil + } + signer.mu.Unlock() + + var creds *credentials.Credentials var err error - signer.onceCredentials.Do(func() { - if signer.credentialsProvider != nil { - signer.credentials, err = signer.credentialsProvider.Get(ctx) - } else if defaultCreds := credentials.Default(); defaultCreds != nil { - signer.credentials = defaultCreds + if signer.credentialsProvider != nil { + creds, err = signer.credentialsProvider.Get(ctx) + if err != nil { + return nil, err } - }) - return signer.credentials, err -} + } else if defaultCreds := credentials.Default(); defaultCreds != nil { + creds = defaultCreds + } -func (signer *signer) onceGetUpToken(ctx context.Context) (string, error) { - signer.onceUpToken.Do(func() { - var credentials *credentials.Credentials - credentials, signer.upTokenErr = signer.onceGetCredentials(ctx) - if signer.upTokenErr != nil { - return - } - var putPolicyJson []byte - putPolicyJson, signer.upTokenErr = json.Marshal(signer.putPolicy) - if signer.upTokenErr != nil { - return - } - signer.upToken = credentials.SignWithData(putPolicyJson) - }) - return signer.upToken, signer.upTokenErr + signer.mu.Lock() + defer signer.mu.Unlock() + if !signer.credentialsInited { + signer.credentials = creds + signer.credentialsInited = true + } + return signer.credentials, nil } // NewParser 创建上传凭证签发器 @@ -115,49 +136,92 @@ func NewParser(upToken string) Provider { } func (parser *parser) GetPutPolicy(context.Context) (PutPolicy, error) { - parser.oncePutPolicy.Do(func() { - splits, ok := parser.onceGetSplits() - if !ok { - parser.putPolicyErr = ErrInvalidUpToken - return - } - var putPolicyJson []byte - putPolicyJson, parser.putPolicyErr = base64.URLEncoding.DecodeString(splits[2]) - if parser.putPolicyErr != nil { - parser.putPolicyErr = ErrInvalidUpToken - return - } - parser.putPolicyErr = json.Unmarshal(putPolicyJson, &parser.putPolicy) - }) - return parser.putPolicy, parser.putPolicyErr + parser.mu.Lock() + if parser.putPolicyInited { + defer parser.mu.Unlock() + return parser.putPolicy, nil + } + parser.mu.Unlock() + + splits, err := parser.getSplits() + if err != nil { + return PutPolicy{}, err + } + + putPolicyJson, err := base64.URLEncoding.DecodeString(splits[2]) + if err != nil { + return PutPolicy{}, ErrInvalidUpToken + } + + var putPolicy PutPolicy + if err := json.Unmarshal(putPolicyJson, &putPolicy); err != nil { + return PutPolicy{}, err + } + + parser.mu.Lock() + defer parser.mu.Unlock() + if !parser.putPolicyInited { + parser.putPolicy = putPolicy + parser.putPolicyInited = true + } + return parser.putPolicy, nil } func (parser *parser) GetAccessKey(context.Context) (string, error) { - parser.onceAccessKey.Do(func() { - splits, ok := parser.onceGetSplits() - if !ok { - parser.accessKeyErr = ErrInvalidUpToken - return - } - parser.accessKey = splits[0] - }) - return parser.accessKey, parser.accessKeyErr + parser.mu.Lock() + if parser.accessKeyInited { + defer parser.mu.Unlock() + return parser.accessKey, nil + } + parser.mu.Unlock() + + splits, err := parser.getSplits() + if err != nil { + return "", err + } + + accessKey := splits[0] + + parser.mu.Lock() + defer parser.mu.Unlock() + if !parser.accessKeyInited { + parser.accessKey = accessKey + parser.accessKeyInited = true + } + return parser.accessKey, nil } -func (parser *parser) onceGetSplits() ([]string, bool) { - parser.onceSplits.Do(func() { - splits := strings.Split(parser.upToken, ":") - if len(splits) == 5 && splits[0] == "" { - splits = splits[2:] +func (parser *parser) getSplits() ([]string, error) { + parser.mu.Lock() + if parser.splitsInited { + defer parser.mu.Unlock() + if parser.splits == nil { + return nil, ErrInvalidUpToken } - if len(splits) != 3 { - parser.splitsValid = false - return + return parser.splits, nil + } + parser.mu.Unlock() + + splits := strings.Split(parser.upToken, ":") + if len(splits) == 5 && splits[0] == "" { + splits = splits[2:] + } + if len(splits) != 3 { + parser.mu.Lock() + defer parser.mu.Unlock() + if !parser.splitsInited { + parser.splitsInited = true } + return nil, ErrInvalidUpToken + } + + parser.mu.Lock() + defer parser.mu.Unlock() + if !parser.splitsInited { parser.splits = splits - parser.splitsValid = true - }) - return parser.splits, parser.splitsValid + parser.splitsInited = true + } + return parser.splits, nil } func (parser *parser) GetUpToken(context.Context) (string, error) {