You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "xlq20080808 (via GitHub)" <gi...@apache.org> on 2023/08/22 04:44:13 UTC

[GitHub] [dubbo] xlq20080808 commented on a diff in pull request #12905: Triple attachment support non-ASCII characters (#12904)

xlq20080808 commented on code in PR #12905:
URL: https://github.com/apache/dubbo/pull/12905#discussion_r1300898182


##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/transport/AbstractH2TransportListener.java:
##########
@@ -63,7 +67,11 @@ protected Map<String, Object> headersToMap(Http2Headers trailers, Supplier<Objec
                     LOGGER.error(PROTOCOL_FAILED_PARSE, "", "", "Failed to parse response attachment key=" + key, e);
                 }
             } else {
-                attachments.put(key, header.getValue().toString());
+                try {
+                    attachments.put(key, URLDecoder.decode(header.getValue().toString(), String.valueOf(StandardCharsets.UTF_8)));
+                } catch (UnsupportedEncodingException e) {
+                    LOGGER.error(PROTOCOL_FAILED_DECODE, "", "", "Failed decode attachment key=" + key, e);
+                }

Review Comment:
   > Check if such is decoded by Dubbo or this may break some user's usage if they want to pass a exact value which is URLEncoded.
   
   I understand that this situation may due to different Dubbo versions of the consumer and provider 
   
   If both the consumer and provider include this pr, when a user wants to pass a URL-encoded value in the consumer, Dubbo will encode it first, and in the provider dubbo will decode it, the user receives the URL-encoded value. This is as expected
   
   However, problems may arise when different dubbo versions are used:
   
   1、If the consumer does not include the pr and the provider included, direct decoding by the provider could cause issues.
   
   2、If the consumer includes the pr and the provider does not include the pr, the provider will receive the encoded value without decoding it. If the consumer attachment includes special values (e.g. '%' or '+'), they will be encoded, resulting in unexpected behavior.
   
   To ensure compatibility between different versions, it may be necessary to add a configuration option in the URL registered by the provider in registration center. This configuration can control whether the consumer-side needs to encoding attachment and provider needs to decoding



##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/transport/AbstractH2TransportListener.java:
##########
@@ -63,7 +67,11 @@ protected Map<String, Object> headersToMap(Http2Headers trailers, Supplier<Objec
                     LOGGER.error(PROTOCOL_FAILED_PARSE, "", "", "Failed to parse response attachment key=" + key, e);
                 }
             } else {
-                attachments.put(key, header.getValue().toString());
+                try {
+                    attachments.put(key, URLDecoder.decode(header.getValue().toString(), String.valueOf(StandardCharsets.UTF_8)));
+                } catch (UnsupportedEncodingException e) {
+                    LOGGER.error(PROTOCOL_FAILED_DECODE, "", "", "Failed decode attachment key=" + key, e);
+                }

Review Comment:
   > Check if such is decoded by Dubbo or this may break some user's usage if they want to pass a exact value which is URLEncoded.
   
   I understand that this situation may due to different Dubbo versions of the consumer and provider 
   
   If both the consumer and provider include this pr, when a user wants to pass a URL-encoded value in the consumer, Dubbo will encode it first, and in the provider dubbo will decode it, the user receives the URL-encoded value. This is as expected
   
   However, problems may arise when different dubbo versions are used:
   
   1、If the consumer does not include the pr and the provider included, direct decoding by the provider could cause issues.
   
   2、If the consumer includes the pr and the provider does not include the pr, the provider will receive the encoded value without decoding it. If the consumer attachment includes special values (e.g. '%' or '+'), they will be encoded, resulting in unexpected behavior.
   
   To ensure compatibility between different versions, it may be necessary to add a configuration option in the URL registered by the provider in registration center. This configuration can control whether the consumer-side needs to encoding attachment and provider needs to decoding



-- 
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