You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "Lcos-000 (via GitHub)" <gi...@apache.org> on 2026/08/18 13:08:46 UTC
[PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Lcos-000 opened a new pull request, #3680:
URL: https://github.com/apache/dubbo-go/pull/3680
### Description
Fixes #3679
`genericServiceFilter.Invoke` (`filter/generic/service_filter.go:79,82`) performed unguarded type assertions on the generic-call arguments:
```go
mtdName := inv.Arguments()[0].(string)
// ...
args := inv.Arguments()[2].([]hessian.Object)
```
`IsGenericInvocation()` (`protocol/invocation/rpcinvocation.go:94-99`) only checks that `MethodName` is `$invoke`/`$invokeAsync` and that `len(Arguments) == 3`; it does **not** validate the element types. A malformed or malicious `$invoke` request with wrong-typed arguments reached these lines and panicked with `interface conversion: interface {} is X, not Y`.
`genericServiceFilter` is in `DefaultServiceFilters` (`common/constant/default.go:67-69`), so this was reachable by any inbound dubbo request with no opt-in required.
This PR replaces the two bare assertions with comma-ok form and returns a `perrors`-wrapped error via `result.RPCResult{Err}` (matching the existing error-return style used at `service_filter.go:92,99`), so a malformed `$invoke` now returns an error instead of panicking.
### Checklist
- [x] I confirm the target branch is `develop`
- [x] I have run `make fmt` to format my code
- [x] I have run `make test` to run local tests
- [x] I have added tests that prove my fix is effective or that my feature works
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Posted by "Lcos-000 (via GitHub)" <gi...@apache.org>.
Lcos-000 commented on PR #3680:
URL: https://github.com/apache/dubbo-go/pull/3680#issuecomment-5328993553
it seem to have a problem with the ci test.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3680:
URL: https://github.com/apache/dubbo-go/pull/3680#discussion_r3804703604
##########
filter/generic/service_filter.go:
##########
@@ -76,10 +76,18 @@ func (f *genericServiceFilter) Invoke(ctx context.Context, invoker base.Invoker,
}
// get real invocation info from the generic invocation
- mtdName := inv.Arguments()[0].(string)
+ // IsGenericInvocation only guarantees len(Arguments)==3, not the element types;
+ // guard the assertions so a malformed $invoke returns an error instead of panicking.
Review Comment:
```suggestion
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3680:
URL: https://github.com/apache/dubbo-go/pull/3680#issuecomment-5328835328
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3680) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3680&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3680&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3680&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3680&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3680&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3680)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3680:
URL: https://github.com/apache/dubbo-go/pull/3680#issuecomment-5328619860
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3680) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3680&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3680&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3680&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3680&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3680&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3680)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3680:
URL: https://github.com/apache/dubbo-go/pull/3680#issuecomment-5329712553
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3680?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 54.85%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b9794f8`](https://app.codecov.io/gh/apache/dubbo-go/commit/b9794f8833eeeccdb4fda138937b7d27a2906d8d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 934 commits behind head on develop.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #3680 +/- ##
===========================================
+ Coverage 46.76% 54.85% +8.08%
===========================================
Files 295 475 +180
Lines 17172 36800 +19628
===========================================
+ Hits 8031 20186 +12155
- Misses 8287 15001 +6714
- Partials 854 1613 +759
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo-go/pull/3680?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3680:
URL: https://github.com/apache/dubbo-go/pull/3680#issuecomment-5329227371
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3680) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3680&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3680&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3680&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3680&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3680&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3680)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3680:
URL: https://github.com/apache/dubbo-go/pull/3680#discussion_r3804903037
##########
filter/generic/service_filter_test.go:
##########
@@ -269,6 +269,55 @@ func TestServiceFilter_InvokeWithUnsupportedGenericMode(t *testing.T) {
}
}
+// TestServiceFilter_InvokeRejectsMalformedArgs ensures a $invoke with wrong-typed
+// arguments returns an error instead of panicking. See service_filter.go:79,82.
Review Comment:
The test comment references specific line numbers in `service_filter.go` (`79,82`), which will drift over time. Prefer referencing the behavior (e.g., “guards arg[0]/arg[2] type assertions”) or linking to an issue/PR instead of line numbers.
##########
filter/generic/service_filter.go:
##########
@@ -76,10 +76,16 @@ func (f *genericServiceFilter) Invoke(ctx context.Context, invoker base.Invoker,
}
// get real invocation info from the generic invocation
- mtdName := inv.Arguments()[0].(string)
+ mtdName, ok := inv.Arguments()[0].(string)
+ if !ok {
+ return &result.RPCResult{Err: perrors.Errorf("$invoke: arg[0] must be string, got %T", inv.Arguments()[0])}
+ }
// types are not required in dubbo-go, for dubbo-go client to dubbo-go server, types could be nil
types := inv.Arguments()[1]
- args := inv.Arguments()[2].([]hessian.Object)
+ args, ok := inv.Arguments()[2].([]hessian.Object)
+ if !ok {
+ return &result.RPCResult{Err: perrors.Errorf("$invoke: arg[2] must be []hessian.Object, got %T", inv.Arguments()[2])}
+ }
Review Comment:
`inv.Arguments()` is called multiple times and indexed repeatedly. Storing the slice once (e.g., `arguments := inv.Arguments()`) makes the code clearer, avoids repeating indexing logic, and guarantees the same snapshot is used for both the assertion and the error message.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(filter/generic): guard $invoke arg type assertions to avoid panic [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl merged PR #3680:
URL: https://github.com/apache/dubbo-go/pull/3680
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org