[GitHub] [jmeter] vlsi commented on a diff in pull request #6010: fix: use UTF-8 as a default encoding in HTTP sampler

["vlsi (via GitHub)" <gi...@apache.org>]

vlsi commented on code in PR #6010:
URL: https://github.com/apache/jmeter/pull/6010#discussion_r1246287119


##########
src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/util/ConversionUtils.java:
##########
@@ -97,20 +102,85 @@ public static String getEncodingFromContentType(String contentType){
     }
 
     /**
-     * Encodes the string according to RFC7578 and RFC3986.
-     * The string is UTF-8 encoded, and non-ASCII bytes are represented as {@code %XX}.
-     * It is close to UrlEncode, however, {@code percentEncode} does not replace space with +.
+     * Encodes strings for {@code multipart/form-data} names and values.
+     * The encoding is {@code "} as {@code %22}, {@code CR} as {@code %0D}, and {@code LF} as {@code %0A}.
+     * Note: {@code %} is not encoded, so it creates ambiguity which might be resolved in a later specification version.
+     * @see <a href="https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart-form-data">Multipart form data specification</a>
+     * @see <a href="https://github.com/whatwg/html/issues/7575">Escaping % in multipart/form-data</a>
      * @param value input value to convert
      * @return converted value
      * @since 5.6
      */
     @API(status = API.Status.MAINTAINED, since = "5.6")
     public static String percentEncode(String value) {
-        try {
-            return new URI(null, null, value, null).toASCIIString();
-        } catch (URISyntaxException e) {
-            throw new IllegalStateException("Can't encode value " + value, e);
+        if (value.indexOf('"') == -1 && value.indexOf('\r') == -1 && value.indexOf('\n') == -1) {
+            return value;
+        }
+        StringBuilder sb = new StringBuilder(value.length() + 2);
+        for (int i = 0; i < value.length(); i++) {
+            char c = value.charAt(i);
+            switch (c) {
+                case '"':
+                    sb.append("%22");
+                    break;
+                case 0x0A:
+                    sb.append("%0A");
+                    break;
+                case 0x0D:
+                    sb.append("%0D");
+                    break;
+                default:
+                    sb.append(c);

Review Comment:
   In theory, `%` should be escaped as well so the server-side can tell the difference between `".txt` and `%22.txt`, however, current Chrome does not escape `%`, so when they change we 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: dev-unsubscribe@jmeter.apache.org

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