Skip to content
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

Support asterisk #14409

Open
wants to merge 2 commits into
base: 3.3
Choose a base branch
from
Open

Support asterisk #14409

wants to merge 2 commits into from

Conversation

qingkaiyan
Copy link

I want consumer with * version to router diffrent version provider which dubbo 2.7 supported
there is two problem

1.nacos dataId not support * SYMBOL
2.consumer with * version invoke Null version provider will block by PermittedSerializationKeeper.checkSerializationPermitted
Because the provider urlParam of version built by consumer has *
Because the null value will not replace the exist urlParam

@qingkaiyan qingkaiyan changed the title 3.3 Support asterisk Jul 9, 2024
Copy link

sonarcloud bot commented Jul 9, 2024

@@ -483,7 +483,7 @@ public String getAnyMethodParameter(String key) {
* @return A new URLParam
*/
public URLParam addParameter(String key, String value) {
if (StringUtils.isEmpty(key) || StringUtils.isEmpty(value)) {
if (StringUtils.isEmpty(key)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this? This may cause some usage mistakes

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a consumer with a * version invoke a null version provider
Building a remote URL based on the consumer's URL will be contiains * version
It's not a good result and will blocked by PermittedSerializationKeeper.checkSerializationPermitted
This issue may only arise when the * symbol is used and The impact of the change will be widespread
But null valus has meanings seems makes scene and 10+ application running well when I made these changes
It's true that no one dares to say these is no other problem
I've been experimenting for over a month and haven't failed yet

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And a consumer with a * version invoke a null version provider worked well at 2.7
I use different version and condition-router to achieve gray and business group
And like to make the dubbo:consumer version = *
It's not work when I update to 3.2 or 3.3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to change PermittedSerializationKeeper.checkSerializationPermitted in the scenarios.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe It's not just this problom
It would be better when the * version consumer called the null version provider to build the null version URL

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URL is a really really basic component for Dubbo, so I would prefer not to change it enless we have the conclusion that we can only change it to fix a issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants