-
Notifications
You must be signed in to change notification settings - Fork 191
fix: fix issue detect by doubao #94
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -55,7 +55,7 @@ func (s *ServerTlsConfigBuilder) BuildTlsConfig() (*tls.Config, error) { | |||||||||||||||||||||
| config *tls.Config | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| if certificate, err = tls.LoadX509KeyPair(s.ServerKeyCertChainPath, s.ServerPrivateKeyPath); err != nil { | ||||||||||||||||||||||
| log.Error(fmt.Sprintf("tls.LoadX509KeyPair(certs{%s}, privateKey{%s}) = err:%+v", | ||||||||||||||||||||||
| _ = log.Error(fmt.Sprintf("tls.LoadX509KeyPair(certs{%s}, privateKey{%s}) = err:%+v", | ||||||||||||||||||||||
| s.ServerKeyCertChainPath, s.ServerPrivateKeyPath, perrors.WithStack(err))) | ||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -68,12 +68,12 @@ func (s *ServerTlsConfigBuilder) BuildTlsConfig() (*tls.Config, error) { | |||||||||||||||||||||
| if s.ServerTrustCertCollectionPath != "" { | ||||||||||||||||||||||
| certPem, err = os.ReadFile(s.ServerTrustCertCollectionPath) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| log.Error(fmt.Errorf("os.ReadFile(certFile{%s}) = err:%+v", s.ServerTrustCertCollectionPath, perrors.WithStack(err))) | ||||||||||||||||||||||
| _ = log.Error(fmt.Errorf("os.ReadFile(certFile{%s}) = err:%+v", s.ServerTrustCertCollectionPath, perrors.WithStack(err))) | ||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| certPool = x509.NewCertPool() | ||||||||||||||||||||||
| if ok := certPool.AppendCertsFromPEM(certPem); !ok { | ||||||||||||||||||||||
| log.Error("failed to parse root certificate file") | ||||||||||||||||||||||
| _ = log.Error("failed to parse root certificate file") | ||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
75
to
78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix returning nil error on server cert parse failure When - if ok := certPool.AppendCertsFromPEM(certPem); !ok {
- _ = log.Error("failed to parse root certificate file")
- return nil, err
+ if ok := certPool.AppendCertsFromPEM(certPem); !ok {
+ err = perrors.New("failed to parse root certificate file")
+ _ = log.Error(err)
+ return nil, err📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| config.ClientCAs = certPool | ||||||||||||||||||||||
|
|
@@ -95,18 +95,18 @@ type ClientTlsConfigBuilder struct { | |||||||||||||||||||||
| func (c *ClientTlsConfigBuilder) BuildTlsConfig() (*tls.Config, error) { | ||||||||||||||||||||||
| cert, err := tls.LoadX509KeyPair(c.ClientKeyCertChainPath, c.ClientPrivateKeyPath) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| log.Error(fmt.Sprintf("Unable to load X509 Key Pair %v", err)) | ||||||||||||||||||||||
| _ = log.Error(fmt.Sprintf("Unable to load X509 Key Pair %v", err)) | ||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| certBytes, err := os.ReadFile(c.ClientTrustCertCollectionPath) | ||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||
| log.Error(fmt.Sprintf("Unable to read pem file: %s", c.ClientTrustCertCollectionPath)) | ||||||||||||||||||||||
| _ = log.Error(fmt.Sprintf("Unable to read pem file: %s", c.ClientTrustCertCollectionPath)) | ||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| clientCertPool := x509.NewCertPool() | ||||||||||||||||||||||
| ok := clientCertPool.AppendCertsFromPEM(certBytes) | ||||||||||||||||||||||
| if !ok { | ||||||||||||||||||||||
| log.Error("failed to parse root certificate") | ||||||||||||||||||||||
| _ = log.Error("failed to parse root certificate") | ||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
108
to
111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Propagate a real error when client cert parsing fails Same issue here: - ok := clientCertPool.AppendCertsFromPEM(certBytes)
- if !ok {
- _ = log.Error("failed to parse root certificate")
- return nil, err
+ ok := clientCertPool.AppendCertsFromPEM(certBytes)
+ if !ok {
+ err = perrors.New("failed to parse root certificate")
+ _ = log.Error(err)
+ return nil, err🤖 Prompt for AI Agents |
||||||||||||||||||||||
| return &tls.Config{ | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,14 +26,29 @@ import ( | |
| type Logger interface { | ||
| Info(args ...any) | ||
| Warn(args ...any) | ||
| Error(args ...any) | ||
| Error(args ...any) error | ||
| Debug(args ...any) | ||
| Infof(fmt string, args ...any) | ||
| Warnf(fmt string, args ...any) | ||
| Errorf(fmt string, args ...any) | ||
| Errorf(fmt string, args ...any) error | ||
| Debugf(fmt string, args ...any) | ||
| } | ||
|
Comment on lines
+29
to
35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 🧩 Analysis chainBreaking change: Logger.Error/Errorf now return error. Provide a backward‑compat shim. This breaks existing custom loggers passed via Proposed additions (outside the hunks): // LegacyLogger mirrors the old contract (no error returns).
type LegacyLogger interface {
Info(args ...any)
Warn(args ...any)
Error(args ...any)
Debug(args ...any)
Infof(string, ...any)
Warnf(string, ...any)
Errorf(string, ...any)
Debugf(string, ...any)
}
// Wrap a legacy logger so it satisfies the new Logger.
type legacyLoggerAdapter struct{ LegacyLogger }
func (l legacyLoggerAdapter) Error(args ...any) error { l.LegacyLogger.Error(args...); return nil }
func (l legacyLoggerAdapter) Errorf(f string, a ...any) error {
l.LegacyLogger.Errorf(f, a...); return nil
}
// SetLegacyLogger allows existing integrations to continue working.
func SetLegacyLogger(legacy LegacyLogger) { log = legacyLoggerAdapter{LegacyLogger: legacy} }Search for external uses in this repo: 🏁 Script executed: #!/bin/bash
rg -nP '\bSetLogger\(' -C2Length of output: 183 Add LegacyLogger adapter for backward compatibility // LegacyLogger mirrors the old contract (no error returns).
type LegacyLogger interface {
Info(args ...any)
Warn(args ...any)
Error(args ...any)
Debug(args ...any)
Infof(string, ...any)
Warnf(string, ...any)
Errorf(string, ...any)
Debugf(string, ...any)
}
// legacyLoggerAdapter wraps a LegacyLogger to satisfy the new Logger.
type legacyLoggerAdapter struct{ LegacyLogger }
func (l legacyLoggerAdapter) Error(args ...any) error {
l.LegacyLogger.Error(args...)
return nil
}
func (l legacyLoggerAdapter) Errorf(f string, a ...any) error {
l.LegacyLogger.Errorf(f, a...)
return nil
}
// SetLegacyLogger installs a LegacyLogger for compatibility with SetLogger.
func SetLegacyLogger(legacy LegacyLogger) {
log = legacyLoggerAdapter{LegacyLogger: legacy}
}🤖 Prompt for AI Agents |
||
|
|
||
| // zapLoggerAdapter adapts zap.SugaredLogger to the Logger interface | ||
| type zapLoggerAdapter struct { | ||
| *zap.SugaredLogger | ||
| } | ||
|
|
||
| func (l *zapLoggerAdapter) Error(args ...any) error { | ||
| l.SugaredLogger.Error(args...) | ||
| return nil | ||
| } | ||
|
|
||
| func (l *zapLoggerAdapter) Errorf(fmt string, args ...any) error { | ||
| l.SugaredLogger.Errorf(fmt, args...) | ||
| return nil | ||
| } | ||
|
|
||
| type LoggerLevel int8 | ||
|
|
||
| const ( | ||
|
|
@@ -79,7 +94,7 @@ var ( | |
| func init() { | ||
| zapLoggerConfig.EncoderConfig = zapLoggerEncoderConfig | ||
| zapLogger, _ = zapLoggerConfig.Build() | ||
| log = zapLogger.Sugar() | ||
| log = &zapLoggerAdapter{zapLogger.Sugar()} | ||
|
|
||
| // todo: flushes buffer when redirect log to file. | ||
| // var exitSignal = make(chan os.Signal) | ||
|
|
@@ -114,7 +129,7 @@ func SetLoggerLevel(level LoggerLevel) error { | |
| if err != nil { | ||
| return err | ||
| } | ||
| log = zapLogger.Sugar() | ||
| log = &zapLoggerAdapter{zapLogger.Sugar()} | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -128,7 +143,7 @@ func SetLoggerCallerDisable() error { | |
| if err != nil { | ||
| return err | ||
| } | ||
| log = zapLogger.Sugar() | ||
| log = &zapLoggerAdapter{zapLogger.Sugar()} | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -163,11 +178,11 @@ func Warnf(template string, args ...any) { | |
| } | ||
|
|
||
| // Error | ||
| func Error(args ...any) { | ||
| log.Error(args...) | ||
| func Error(args ...any) error { | ||
| return log.Error(args...) | ||
| } | ||
|
|
||
| // Errorf | ||
| func Errorf(template string, args ...any) { | ||
| log.Errorf(template, args...) | ||
| func Errorf(template string, args ...any) error { | ||
| return log.Errorf(template, args...) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New closeWait channel isn’t reinitialized in Reset(); close(nil) panic risk.
handlePackageclosess.closeWait. AfterReset(),closeWaitbecomes nil (zero value) and closing it will panic.Apply this diff to
Reset():func (s *session) Reset() { *s = session{ name: defaultSessionName, once: &sync.Once{}, done: make(chan struct{}), period: period, wait: pendingDuration, - attrs: gxcontext.NewValuesContext(context.Background()), + attrs: gxcontext.NewValuesContext(context.Background()), + closeWait: make(chan struct{}), } }Also applies to: 161-166
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good advice