ok2c commented on code in PR #517:
URL: 
https://github.com/apache/httpcomponents-core/pull/517#discussion_r1996675961


##########
httpcore5/src/main/java/org/apache/hc/core5/net/PercentCodec.java:
##########
@@ -113,6 +113,37 @@ public class PercentCodec {
         RFC5987_UNRESERVED.set('~');
     }
 
+    static final BitSet PCHAR = new BitSet(256);
+    static final BitSet USERINFO = new BitSet(256);
+    static final BitSet REG_NAME = new BitSet(256);
+    static final BitSet PATH_SEGMENT = new BitSet(256);
+    static final BitSet QUERY = new BitSet(256);
+    static final BitSet FRAGMENT = new BitSet(256);
+
+    static {
+        PCHAR.or(UNRESERVED);
+        PCHAR.or(SUB_DELIMS);
+        PCHAR.set(':');
+        PCHAR.set('@');
+        USERINFO.or(UNRESERVED);
+        USERINFO.or(SUB_DELIMS);
+        USERINFO.set(':');
+        REG_NAME.or(UNRESERVED);
+        REG_NAME.or(SUB_DELIMS);
+        REG_NAME.clear('!');
+        PATH_SEGMENT.or(PCHAR);
+        QUERY.or(PCHAR);
+        QUERY.set('/');
+        QUERY.set('?');
+        FRAGMENT.or(PCHAR);
+        FRAGMENT.set('/');
+        FRAGMENT.set('?');
+        // Some sub-delims remain encoded (RFC 3986 allows them unencoded, but 
we choose to be strict).
+        PATH_SEGMENT.clear('(');
+        PATH_SEGMENT.clear(')');
+        PATH_SEGMENT.clear('&');
+    }

Review Comment:
   @arturobernalg You have overlooked static `formatQuery` and `formatPath` 
methods. They should also take the encoding policy into account. Apply this 
patch and everything should work
   
   ```
   ===================================================================
   diff --git 
a/httpcore5/src/main/java/org/apache/hc/core5/net/PercentCodec.java 
b/httpcore5/src/main/java/org/apache/hc/core5/net/PercentCodec.java
   --- a/httpcore5/src/main/java/org/apache/hc/core5/net/PercentCodec.java      
(revision 53477ecc93d331f8348a6748a918d86b25666471)
   +++ b/httpcore5/src/main/java/org/apache/hc/core5/net/PercentCodec.java      
(date 1742025384945)
   @@ -137,10 +137,6 @@
            FRAGMENT.or(PCHAR);
            FRAGMENT.set('/');
            FRAGMENT.set('?');
   -        // Some sub-delims remain encoded (RFC 3986 allows them unencoded, 
but we choose to be strict).
   -        PATH_SEGMENT.clear('(');
   -        PATH_SEGMENT.clear(')');
   -        PATH_SEGMENT.clear('&');
        }
    
        private static final int RADIX = 16;
   Index: httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
   IDEA additional info:
   Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
   <+>UTF-8
   ===================================================================
   diff --git a/httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java 
b/httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java
   --- a/httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java        
(revision 53477ecc93d331f8348a6748a918d86b25666471)
   +++ b/httpcore5/src/main/java/org/apache/hc/core5/net/URIBuilder.java        
(date 1742026397263)
   @@ -34,6 +34,7 @@
    import java.nio.charset.StandardCharsets;
    import java.util.ArrayList;
    import java.util.Arrays;
   +import java.util.BitSet;
    import java.util.Collections;
    import java.util.LinkedList;
    import java.util.List;
   @@ -343,19 +344,25 @@
            return list;
        }
    
   -    static void formatPath(final StringBuilder buf, final Iterable<String> 
segments, final boolean rootless, final Charset charset) {
   +    static void formatPath(final StringBuilder buf, final Iterable<String> 
segments, final boolean rootless,
   +                           final Charset charset, final BitSet safechars) {
            int i = 0;
            for (final String segment : segments) {
                if (i > 0 || !rootless) {
                    buf.append(PATH_SEPARATOR);
                }
   -            PercentCodec.encode(buf, segment, charset, 
PercentCodec.PATH_SEGMENT, false);
   +            PercentCodec.encode(buf, segment, charset, safechars, false);
                i++;
            }
        }
    
   -    static void formatQuery(final StringBuilder buf, final Iterable<? 
extends NameValuePair> params, final Charset charset,
   -                            final boolean blankAsPlus) {
   +    static void formatPath(final StringBuilder buf, final Iterable<String> 
segments, final boolean rootless,
   +                           final Charset charset) {
   +        formatPath(buf, segments, rootless, charset, 
PercentCodec.UNRESERVED);
   +    }
   +
   +    static void formatQuery(final StringBuilder buf, final Iterable<? 
extends NameValuePair> params,
   +                            final Charset charset, final BitSet safechars, 
final boolean blankAsPlus) {
            int i = 0;
            for (final NameValuePair parameter : params) {
                if (i > 0) {
   @@ -364,12 +371,17 @@
                PercentCodec.encode(buf, parameter.getName(), charset, 
blankAsPlus);
                if (parameter.getValue() != null) {
                    buf.append(PARAM_VALUE_SEPARATOR);
   -                PercentCodec.encode(buf, parameter.getValue(), charset, 
blankAsPlus);
   +                PercentCodec.encode(buf, parameter.getValue(), charset, 
safechars, blankAsPlus);
                }
                i++;
            }
        }
    
   +    static void formatQuery(final StringBuilder buf, final Iterable<? 
extends NameValuePair> params,
   +                            final Charset charset, final boolean 
blankAsPlus) {
   +        formatQuery(buf, params, charset, PercentCodec.UNRESERVED, 
blankAsPlus);
   +    }
   +
        /**
         * Builds a {@link URI} instance.
         */
   @@ -429,13 +441,15 @@
                    }
                    sb.append(this.encodedPath);
                } else if (this.pathSegments != null) {
   -                formatPath(sb, this.pathSegments, !authoritySpecified && 
this.pathRootless, this.charset);
   +                formatPath(sb, this.pathSegments, !authoritySpecified && 
this.pathRootless, this.charset,
   +                        encodingPolicy == EncodingPolicy.ALL_RESERVED ? 
PercentCodec.UNRESERVED : PercentCodec.PATH_SEGMENT);
                }
                if (this.encodedQuery != null) {
                    sb.append("?").append(this.encodedQuery);
                } else if (this.queryParams != null && 
!this.queryParams.isEmpty()) {
                    sb.append("?");
   -                formatQuery(sb, this.queryParams, this.charset, false);
   +                formatQuery(sb, this.queryParams, this.charset,
   +                        encodingPolicy == EncodingPolicy.ALL_RESERVED ? 
PercentCodec.UNRESERVED : PercentCodec.QUERY, false);
                } else if (this.query != null) {
                    sb.append("?");
                    PercentCodec.encode(sb, this.query, this.charset,
   ```
   



-- 
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: dev-unsubscr...@hc.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org

Reply via email to