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/12/05 06:50:44 UTC
[GitHub] [dubbo] twz007 opened a new pull request, #11072: fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059
twz007 opened a new pull request, #11072:
URL: https://github.com/apache/dubbo/pull/11072
## What is the purpose of the change
#11059
## Brief changelog
## Verifying this change
<!-- Follow this checklist to help us incorporate your contribution quickly and easily: -->
## 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.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] 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.
- [ ] 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.
- [ ] 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] twz007 commented on a diff in pull request #11072: fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059
Posted by GitBox <gi...@apache.org>.
twz007 commented on code in PR #11072:
URL: https://github.com/apache/dubbo/pull/11072#discussion_r1040339735
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/stream/TripleServerStream.java:
##########
@@ -336,12 +336,21 @@ private void processHeader(Http2Headers headers, boolean endStream) {
}
String[] parts = path.split("/");
- if (parts.length != 3) {
+ String serviceName;
+ String originalMethodName;
+ if (parts.length == 3) {
+ serviceName = parts[1];
+ originalMethodName = parts[2];
+ } else if (parts.length == 4) {
+ if (LOGGER.isWarnEnabled()) {
+ LOGGER.warn(String.format("context-path: [%s] will be ignored in the triple protocol, please remove this configuration.", parts[1]));
+ }
+ serviceName = parts[2];
+ originalMethodName = parts[3];
+ } else {
Review Comment:
I don't understand what the intermediate proxies refers to. Do you mean that it is better to modify it on the client side?
--
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] fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059 [dubbo]
Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on PR #11072:
URL: https://github.com/apache/dubbo/pull/11072#issuecomment-1877009475
@twz007 Can you recommit to the 3.2 branch? 3.1 no longer makes this convenient change.
--
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] guohao commented on a diff in pull request #11072: fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059
Posted by GitBox <gi...@apache.org>.
guohao commented on code in PR #11072:
URL: https://github.com/apache/dubbo/pull/11072#discussion_r1039475942
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/stream/TripleServerStream.java:
##########
@@ -336,12 +336,21 @@ private void processHeader(Http2Headers headers, boolean endStream) {
}
String[] parts = path.split("/");
- if (parts.length != 3) {
+ String serviceName;
+ String originalMethodName;
+ if (parts.length == 3) {
+ serviceName = parts[1];
+ originalMethodName = parts[2];
+ } else if (parts.length == 4) {
+ if (LOGGER.isWarnEnabled()) {
+ LOGGER.warn(String.format("context-path: [%s] will be ignored in the triple protocol, please remove this configuration.", parts[1]));
+ }
+ serviceName = parts[2];
+ originalMethodName = parts[3];
+ } else {
Review Comment:
Do fallback here may looks work from remote side but failed on intermediate proxies.
May it be better to just log and `responseErr`?
--
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] twz007 commented on pull request #11072: fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059
Posted by GitBox <gi...@apache.org>.
twz007 commented on PR #11072:
URL: https://github.com/apache/dubbo/pull/11072#issuecomment-1340292292
> > > It is better not to do compatibility at the server protocol layer, it is better to modify when reading the configuration in the client framework as explained in the issue
> >
> >
> > What consumers read is the `context-path` registered by the provider. It seems a bit awkward for consumers to be compatible with the provider. What we want is to let the provider modify their configuration instead of hiding it, or manually to inform them that there is a problem with the configuration.
>
> Modifying from the provider side somewhat breaks the semantics of the protocol, and it doesn’t make sense for the provider side to print warn all the time, we hope consumers can find this problem by themselves.
The log printing is the same for consumers. If the consumer has not called the problematic producer, the log will not be printed or the provider has very little traffic. The problem will be very hidden and may not be exposed until the production goes online.
--
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 #11072: fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11072:
URL: https://github.com/apache/dubbo/pull/11072#issuecomment-1336859937
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=11072)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11072&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11072&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=11072&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=11072&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11072&metric=new_coverage&view=list) [28.6% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11072&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11072&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=11072&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] twz007 commented on pull request #11072: fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059
Posted by GitBox <gi...@apache.org>.
twz007 commented on PR #11072:
URL: https://github.com/apache/dubbo/pull/11072#issuecomment-1338614004
> It is better not to do compatibility at the server protocol layer, it is better to modify when reading the configuration in the client framework as explained in the issue
What consumers read is the `context-path` registered by the provider. It seems a bit awkward for consumers to be compatible with the provider. What we want is to let the provider modify their configuration instead of hiding it, or manually to inform them that there is a problem with the configuration.
--
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] EarthChen commented on pull request #11072: fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059
Posted by GitBox <gi...@apache.org>.
EarthChen commented on PR #11072:
URL: https://github.com/apache/dubbo/pull/11072#issuecomment-1338660653
> > It is better not to do compatibility at the server protocol layer, it is better to modify when reading the configuration in the client framework as explained in the issue
>
> What consumers read is the `context-path` registered by the provider. It seems a bit awkward for consumers to be compatible with the provider. What we want is to let the provider modify their configuration instead of hiding it, or manually to inform them that there is a problem with the configuration.
Modifying from the provider side somewhat breaks the semantics of the protocol, and it doesn’t make sense for the provider side to print warn all the time, we hope consumers can find this problem by themselves.
--
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] guohao commented on a diff in pull request #11072: fixed tri protocol with declare context-path, consumer got a "Bad path format xxx" exception #11059
Posted by GitBox <gi...@apache.org>.
guohao commented on code in PR #11072:
URL: https://github.com/apache/dubbo/pull/11072#discussion_r1039475942
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/stream/TripleServerStream.java:
##########
@@ -336,12 +336,21 @@ private void processHeader(Http2Headers headers, boolean endStream) {
}
String[] parts = path.split("/");
- if (parts.length != 3) {
+ String serviceName;
+ String originalMethodName;
+ if (parts.length == 3) {
+ serviceName = parts[1];
+ originalMethodName = parts[2];
+ } else if (parts.length == 4) {
+ if (LOGGER.isWarnEnabled()) {
+ LOGGER.warn(String.format("context-path: [%s] will be ignored in the triple protocol, please remove this configuration.", parts[1]));
+ }
+ serviceName = parts[2];
+ originalMethodName = parts[3];
+ } else {
Review Comment:
Do fallback here may look work from remote side but fails on intermediate proxies.
May it be better to just log and `responseErr`?
--
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