You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2022/11/11 12:06:22 UTC
[GitHub] [dubbo] wxbty opened a new pull request, #10921: fix NPE bug of URL
wxbty opened a new pull request, #10921:
URL: https://github.com/apache/dubbo/pull/10921
What is the purpose of the change
When the org.apache.dubbo.common.URL uses setHost, setPassword, there is an NPE risk of the urlAddress property, because the URL has an empty constructor. For example, when zk pushes the url to the listener, it will call the DefaultServiceInstance.toURL method to construct the URL
@Override
public InstanceAddressURL toURL(String protocol) {
if (instanceAddressURL == null) {
instanceAddressURL = new InstanceAddressURL(this, serviceMetadata, protocol);
}
return instanceAddressURL;
}
The constructor of InstanceAddressURL here only sets its own properties, which is equivalent to calling the empty constructor of the parent class URL. The constructed URL instance urlAddress property is empty. When someone calls the setHost of this URL, an NPE error will be reported, such as this bug #issue https://github.com/apache/dubbo-spi-extensions/issues/166
The root cause of this error is the dubbo URL design problem, dubbo-spi-extensions did not judge, and then stepped into the trap.
Brief changelog
Verifying this change
Checklist
- [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
- [ ] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in [dubbo samples](https://github.com/apache/dubbo-samples) project.
- [x] Add some description to [dubbo-website](https://github.com/apache/dubbo-website) project if you are requesting to add a feature.
- [ ] GitHub Actions works fine on your own branch.
- [x] If this contribution is large, please follow the [Software Donation Guide](https://github.com/apache/dubbo/wiki/Software-donation-guide).
--
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
[GitHub] [dubbo] wxbty commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
wxbty commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1313525389
> BTW, this issue may be already fixed in #10256.
> BTW, this issue may be already fixed in #10256.
I noticed this change, it will be fixed in version 3.1, what I want to show is that the parent URL needs to guarantee its NPE check, so that no matter what, Exception won't happen on the path of the URL.
If URL defines a new subclass, the author will notice that setHost may return null, and he needs to handle this exception. With the current way of writing, he will think that the URL must be returned.
I think, all base class methods should consider null scenarios.
--
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
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on code in PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#discussion_r1023648421
##########
dubbo-common/src/main/java/org/apache/dubbo/common/URL.java:
##########
@@ -499,8 +529,15 @@ public String getPath() {
}
public URL setPath(String path) {
- URLAddress newURLAddress = urlAddress.setPath(path);
- return returnURL(newURLAddress);
+ URL url;
+ if (urlAddress == null) {
+ url = new URL();
+ url.setPath(path);
+ } else {
+ URLAddress newURLAddress = urlAddress.setPath(path);
+ url = returnURL(newURLAddress);
+ }
+ return url;
Review Comment:
```suggestion
if (urlAddress == null) {
return new ServiceConfigURL(getProtocol(), getHost(), getPort(), path, getParameters());
} else {
URLAddress newURLAddress = urlAddress.setPath(path);
return returnURL(newURLAddress);
}
```
##########
dubbo-common/src/main/java/org/apache/dubbo/common/URL.java:
##########
@@ -425,8 +433,15 @@ public String getHost() {
}
public URL setHost(String host) {
- URLAddress newURLAddress = urlAddress.setHost(host);
- return returnURL(newURLAddress);
+ URL url;
+ if (urlAddress == null) {
+ url = new URL();
+ url.setHost(host);
+ } else {
+ URLAddress newURLAddress = urlAddress.setHost(host);
+ url = returnURL(newURLAddress);
+ }
+ return url;
Review Comment:
```suggestion
```suggestion
if (urlAddress == null) {
return new ServiceConfigURL(getProtocol(), host, getPort(), getPath(), getParameters());
} else {
URLAddress newURLAddress = urlAddress.setHost(host);
return returnURL(newURLAddress);
}
```
```
##########
dubbo-common/src/main/java/org/apache/dubbo/common/URL.java:
##########
@@ -458,12 +480,20 @@ public URL setAddress(String address) {
} else {
host = address;
}
- URLAddress newURLAddress = urlAddress.setAddress(host, port);
- return returnURL(newURLAddress);
+ URL url;
+ if (urlAddress == null) {
+ url = new URL();
+ url.setHost(host);
+ url.setPort(port);
+ } else {
+ URLAddress newURLAddress = urlAddress.setAddress(host, port);
+ url = returnURL(newURLAddress);
+ }
+ return url;
Review Comment:
```suggestion
if (urlAddress == null) {
return new ServiceConfigURL(getProtocol(), host, port, getPath(), getParameters());
} else {
URLAddress newURLAddress = urlAddress.setAddress(host, port);
return returnURL(newURLAddress);
}
```
##########
dubbo-common/src/main/java/org/apache/dubbo/common/URL.java:
##########
@@ -435,8 +450,15 @@ public int getPort() {
}
public URL setPort(int port) {
- URLAddress newURLAddress = urlAddress.setPort(port);
- return returnURL(newURLAddress);
+ URL url;
+ if (urlAddress == null) {
+ url = new URL();
+ url.setPort(port);
+ } else {
+ URLAddress newURLAddress = urlAddress.setPort(port);
+ url = returnURL(newURLAddress);
+ }
+ return url;
Review Comment:
```suggestion
if (urlAddress == null) {
return new ServiceConfigURL(getProtocol(), getHost(), port, getPath(), getParameters());
} else {
URLAddress newURLAddress = urlAddress.setPort(port);
return returnURL(newURLAddress);
}
```
--
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
[GitHub] [dubbo] AlbumenJ commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1313293407
https://github.com/apache/dubbo/pull/10256/files#diff-8cea6b30b1f89caf0117eb66c7abca7e86e46ecb1f232a8508290ab40ca70ffd
--
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
[GitHub] [dubbo] AlbumenJ commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1313221504
> > Let `InstanceAddressURL` override these methods would be better
>
> If urlAddress is empty, does it simply return null? I think we need the necessary NPE check for the URL, which has other subclasses.
In `InstanceAddressURL`, a new URL instance should be created if there are some modifications. The new URL type can be `DubboServiceAddressURL`.
The case of `urlParam` is null is only happends in `InstanceAddressURL`, and override those origin method would fix.
--
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
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on code in PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#discussion_r1025906142
##########
dubbo-common/src/main/java/org/apache/dubbo/common/URL.java:
##########
@@ -367,8 +367,16 @@ public String getPassword() {
}
public URL setPassword(String password) {
- URLAddress newURLAddress = urlAddress.setPassword(password);
- return returnURL(newURLAddress);
+
+ URL url;
+ if (urlAddress == null) {
+ url = new URL();
+ url.setPassword(password);
+ } else {
+ URLAddress newURLAddress = urlAddress.setPassword(password);
+ url = returnURL(newURLAddress);
+ }
+ return url;
Review Comment:
```suggestion
if (urlAddress == null) {
return new ServiceConfigURL(getProtocol(), getHost(), port, getPath(), getParameters()).setPassword(password);
} else {
URLAddress newURLAddress = urlAddress.setPassword(password);
return returnURL(newURLAddress);
}
```
##########
dubbo-common/src/main/java/org/apache/dubbo/common/URL.java:
##########
@@ -425,8 +433,13 @@ public String getHost() {
}
public URL setHost(String host) {
- URLAddress newURLAddress = urlAddress.setHost(host);
- return returnURL(newURLAddress);
+```suggestion
Review Comment:
remove this breaking line
##########
dubbo-common/src/main/java/org/apache/dubbo/common/URL.java:
##########
@@ -367,8 +367,16 @@ public String getPassword() {
}
public URL setPassword(String password) {
- URLAddress newURLAddress = urlAddress.setPassword(password);
- return returnURL(newURLAddress);
+
+ URL url;
+ if (urlAddress == null) {
+ url = new URL();
+ url.setPassword(password);
+ } else {
+ URLAddress newURLAddress = urlAddress.setPassword(password);
+ url = returnURL(newURLAddress);
+ }
+ return url;
Review Comment:
BTW, please fix `setUsername`, `setProtocol` and etc
--
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
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1312965982
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=10921)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=duplicated_lines_density&view=list) No Duplication information
--
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
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1319603125
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=10921)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_coverage&view=list) [47.1% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dubbo] wxbty closed pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
wxbty closed pull request #10921: fix NPE bug of URL
URL: https://github.com/apache/dubbo/pull/10921
--
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
[GitHub] [dubbo] wxbty commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
wxbty commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1312652202
> Let `InstanceAddressURL` override these methods would be better
If urlAddress is empty, does it simply return null? I think we need the necessary NPE check for the URL, which has other subclasses.
--
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
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1321426505
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=10921)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_coverage&view=list) [65.4% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dubbo] AlbumenJ commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1313292527
BTW, this issue may be already fixed in #10256.
--
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
[GitHub] [dubbo] wxbty commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
wxbty commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1312958241
hi, @AlbumenJ
Do you think this modification is reasonable? The URL constructor does not use mandatory assignment attributes, so null checking is necessary, the base class should be more robust, and dubbo's extension project dependencies are confusing, and relying on subclasses may still occur frequently NPE.
--
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
[GitHub] [dubbo] chickenlj commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
chickenlj commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1312420139
Yeah, I agree with Albumen
--
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
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1312974637
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=10921)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=duplicated_lines_density&view=list) No Duplication information
--
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
[GitHub] [dubbo] wxbty commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
wxbty commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1313264110
> Do you think this modification is reasonable? The URL constructor does not use mandatory assignment attributes, so null checking is necessary, the base class should be more robust, and dubbo's extension project dependencies are confusing, and relying on subclasses may still occur frequently NPE.
If you think don't need to modify it, I will close it. But still feel that there is a risk of NPE like this, which looks badly designed
--
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
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1312408581
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=10921)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=duplicated_lines_density&view=list) No Duplication information
--
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
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1313346456
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=10921)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=duplicated_lines_density&view=list) No Duplication information
--
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
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1316204500
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=10921)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_coverage&view=list) [46.9% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dubbo] AlbumenJ commented on a diff in pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on code in PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#discussion_r1027443693
##########
dubbo-common/src/main/java/org/apache/dubbo/common/URL.java:
##########
@@ -349,26 +349,38 @@ public String getProtocol() {
}
public URL setProtocol(String protocol) {
- URLAddress newURLAddress = urlAddress.setProtocol(protocol);
- return returnURL(newURLAddress);
+ if (urlAddress == null) {
+ return new ServiceConfigURL(getProtocol(), getHost(), getPort(), getPath(), getParameters());
Review Comment:
```suggestion
return new ServiceConfigURL(protocol, getHost(), getPort(), getPath(), getParameters());
```
--
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
[GitHub] [dubbo] codecov-commenter commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1313356938
# [Codecov](https://codecov.io/gh/apache/dubbo/pull/10921?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10921](https://codecov.io/gh/apache/dubbo/pull/10921?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9096853) into [3.1](https://codecov.io/gh/apache/dubbo/commit/b6849441a5c6ae79cce36147521cedceb8f26f86?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b684944) will **decrease** coverage by `4.09%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## 3.1 #10921 +/- ##
============================================
- Coverage 68.99% 64.90% -4.10%
+ Complexity 561 14 -547
============================================
Files 1365 1421 +56
Lines 72749 58890 -13859
Branches 12794 8646 -4148
============================================
- Hits 50191 38220 -11971
+ Misses 18044 16656 -1388
+ Partials 4514 4014 -500
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo/pull/10921?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...mon/src/main/java/org/apache/dubbo/common/URL.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vVVJMLmphdmE=) | `44.45% <0.00%> (-6.50%)` | :arrow_down: |
| [...luster/router/script/ScriptStateRouterFactory.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL3NjcmlwdC9TY3JpcHRTdGF0ZVJvdXRlckZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...zookeeper/curator/CuratorZookeeperTransporter.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3Rpbmctem9va2VlcGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy96b29rZWVwZXIvY3VyYXRvci9DdXJhdG9yWm9va2VlcGVyVHJhbnNwb3J0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pc/cluster/router/file/FileStateRouterFactory.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL2ZpbGUvRmlsZVN0YXRlUm91dGVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-80.96%)` | :arrow_down: |
| [...o/rpc/cluster/router/script/ScriptStateRouter.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvcm91dGVyL3NjcmlwdC9TY3JpcHRTdGF0ZVJvdXRlci5qYXZh) | `0.00% <0.00%> (-73.34%)` | :arrow_down: |
| [...mmon/serialize/fastjson2/FastJson2ObjectInput.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tc2VyaWFsaXphdGlvbi9kdWJiby1zZXJpYWxpemF0aW9uLWZhc3Rqc29uMi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vY29tbW9uL3NlcmlhbGl6ZS9mYXN0anNvbjIvRmFzdEpzb24yT2JqZWN0SW5wdXQuamF2YQ==) | `0.00% <0.00%> (-62.50%)` | :arrow_down: |
| [...ting/zookeeper/curator/CuratorZookeeperClient.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3Rpbmctem9va2VlcGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy96b29rZWVwZXIvY3VyYXRvci9DdXJhdG9yWm9va2VlcGVyQ2xpZW50LmphdmE=) | `15.38% <0.00%> (-61.73%)` | :arrow_down: |
| [...mon/serialize/fastjson2/FastJson2ObjectOutput.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tc2VyaWFsaXphdGlvbi9kdWJiby1zZXJpYWxpemF0aW9uLWZhc3Rqc29uMi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vY29tbW9uL3NlcmlhbGl6ZS9mYXN0anNvbjIvRmFzdEpzb24yT2JqZWN0T3V0cHV0LmphdmE=) | `0.00% <0.00%> (-56.25%)` | :arrow_down: |
| [...org/apache/dubbo/remoting/api/pu/DefaultCodec.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9hcGkvcHUvRGVmYXVsdENvZGVjLmphdmE=) | `33.33% <0.00%> (-33.34%)` | :arrow_down: |
| [...dubbo/remoting/zookeeper/ZookeeperTransporter.java](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy96b29rZWVwZXIvWm9va2VlcGVyVHJhbnNwb3J0ZXIuamF2YQ==) | `50.00% <0.00%> (-33.34%)` | :arrow_down: |
| ... and [489 more](https://codecov.io/gh/apache/dubbo/pull/10921/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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
[GitHub] [dubbo] AlbumenJ commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1313292082
> If you think don't need to modify it, I will close it. But still feel that there is a risk of NPE like this, which looks badly designed
Bugfix is needed.
To make it clarify, I will give some suggest changes for it.
```java
public class InstanceAddressURL {
@Override
public URL setHost(String host) {
// copy current URL into a new DubboServiceAddressURL
DubboServiceAddressURL copyOf = createDubboServiceAddressURL(this);
return copyOf.setHost(host);
}
}
```
--
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
[GitHub] [dubbo] wxbty commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
wxbty commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1315349444
> > I noticed this change, it will be fixed in version 3.1, what I want to show is that the parent URL needs to guarantee its NPE check, so that no matter what, Exception won't happen on the path of the URL. If URL defines a new subclass, the author will notice that setHost may return null, and he needs to handle this exception. With the current way of writing, he will think that the URL must be returned. I think, all base class methods should consider null scenarios.
>
> The original solution can indeed solve the NPE problem to a certain extent. (You can reset this PR to the original commit, and I will merge it later.)
>
> In the future, we hope to turn the URL object into an interface at an appropriate time(may be 3.2). The abstract class URL no longer provides attribute storage and implementation, but only provides interface definition. Based on this, Dubbo's URL system is refactored to avoid the current need for various overriding methods. (Would you please help do this?)
Ok, I'll be happy to do it
--
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
[GitHub] [dubbo] sonarcloud[bot] commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1319604847
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=10921)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=10921&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=10921&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_coverage&view=list) [47.1% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=10921&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dubbo] AlbumenJ commented on pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ commented on PR #10921:
URL: https://github.com/apache/dubbo/pull/10921#issuecomment-1314653712
> I noticed this change, it will be fixed in version 3.1, what I want to show is that the parent URL needs to guarantee its NPE check, so that no matter what, Exception won't happen on the path of the URL. If URL defines a new subclass, the author will notice that setHost may return null, and he needs to handle this exception. With the current way of writing, he will think that the URL must be returned. I think, all base class methods should consider null scenarios.
The original solution can indeed solve the NPE problem to a certain extent. (You can reset this PR to the original commit, and I will merge it later.)
In the future, we hope to turn the URL object into an interface at an appropriate time(may be 3.2). The abstract class URL no longer provides attribute storage and implementation, but only provides interface definition.
Based on this, Dubbo's URL system is refactored to avoid the current need for various overriding methods. (Would you please help do this?)
--
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
[GitHub] [dubbo] AlbumenJ merged pull request #10921: fix NPE bug of URL
Posted by GitBox <gi...@apache.org>.
AlbumenJ merged PR #10921:
URL: https://github.com/apache/dubbo/pull/10921
--
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