AlexStocks commented on PR #3164:
URL: https://github.com/apache/dubbo-go/pull/3164#issuecomment-4059396686
Post-merge review,两个问题:
## 1. `url.SubURL` 可能为 nil,直接解引用会 panic
`cluster/router/polaris/router.go:L58-L63`:
```go
if applicationName == "" {
applicationName = url.SubURL.GetParam(constant.ApplicationKey, "") //
SubURL 可能 nil
}
```
`SubURL` 是 `*common.URL` 指针类型,虽然在当前 registry directory 场景下通常不为空,但其他路径(如直接构造
URL)调用 `newPolarisRouter` 时会 panic。应该加 nil check:
```go
if applicationName == "" && url.SubURL != nil {
applicationName = url.SubURL.GetParam(constant.ApplicationKey, "")
}
```
## 2. PR 没有修复根因——attribute vs parameter 存储不匹配
`registry/directory/directory.go:L88-96` 把 `ApplicationConfig` 作为**结构体对象**存到
URL attribute 里:
```go
url.SetAttribute(constant.ApplicationKey, application) //
*global.ApplicationConfig
```
但 polaris router 始终用 `url.GetParam` 读 URL **parameter**(字符串),两者永远对不上。这个 PR
只是多加了一层 `SubURL.GetParam` 的 fallback,如果 SubURL 里也没有 ApplicationKey 参数,问题依然存在。
正确的修法应该同时从 URL attribute 读取 `*global.ApplicationConfig` 对象(codebase 里已有这个
pattern,如 `directory.go:L223`):
```go
applicationName := url.GetParam(constant.ApplicationKey, "")
if applicationName == "" {
if appConfRaw, ok := url.GetAttribute(constant.ApplicationKey); ok {
if appConf, ok := appConfRaw.(*global.ApplicationConfig); ok &&
appConf != nil {
applicationName = appConf.Name
}
}
}
if applicationName == "" && url.SubURL != nil {
applicationName = url.SubURL.GetParam(constant.ApplicationKey, "")
if applicationName == "" {
if appConfRaw, ok :=
url.SubURL.GetAttribute(constant.ApplicationKey); ok {
if appConf, ok := appConfRaw.(*global.ApplicationConfig); ok &&
appConf != nil {
applicationName = appConf.Name
}
}
}
}
if applicationName == "" {
return nil, fmt.Errorf("polaris router must set application name")
}
```
建议开 follow-up PR 修复。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]