ok2c commented on code in PR #618:
URL: 
https://github.com/apache/httpcomponents-client/pull/618#discussion_r1970401181


##########
httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/FormBodyPartBuilder.java:
##########
@@ -96,12 +99,61 @@ public FormBodyPartBuilder setField(final String name, 
final String value) {
         return this;
     }
 
+    /**
+     * Sets the multipart mode for this builder, determining how filenames are 
encoded in the
+     * {@code Content-Disposition} header. The mode affects whether the {@code 
filename*} parameter
+     * is included for non-ISO-8859-1 filenames.
+     * <p>
+     * In {@link HttpMultipartMode#LEGACY} mode, only the {@code filename} 
parameter is included
+     * with the raw value, mimicking pre-RFC 7578 behavior. In {@link 
HttpMultipartMode#STRICT}
+     * or {@link HttpMultipartMode#EXTENDED} modes, the {@code filename*} 
parameter is added
+     * with RFC 5987 encoding for filenames containing non-ISO-8859-1 
characters.
+     * </p>
+     * <p>
+     * If {@code mode} is {@code null}, it defaults to {@link 
HttpMultipartMode#STRICT}.
+     * </p>
+     *
+     * @param mode the {@link HttpMultipartMode} to use, or {@code null} for 
default behavior
+     * @return this builder instance for method chaining
+     * @since 5.5
+     */
+    public FormBodyPartBuilder setMode(final HttpMultipartMode mode) {
+        this.mode = mode != null ? mode : HttpMultipartMode.STRICT;
+        return this;
+    }
+
     public FormBodyPartBuilder removeFields(final String name) {
         Args.notNull(name, "Field name");
         this.header.removeFields(name);
         return this;
     }
 
+    /**
+     * Determines whether the given string can be encoded in ISO-8859-1 
without loss of data.
+     * This is used to decide whether the {@code filename} parameter can be 
used as-is or if
+     * the {@code filename*} parameter is needed for non-ISO-8859-1 characters.
+     *
+     * @param input the string to check, must not be {@code null}
+     * @return {@code true} if the string can be encoded in ISO-8859-1, {@code 
false} otherwise
+     * @since 5.5
+     */
+    private static boolean canEncodeToISO8859_1(final String input) {
+        return StandardCharsets.ISO_8859_1.newEncoder().canEncode(input);

Review Comment:
   @arturobernalg Let's make the method non-static re-use the encoder rather 
than creating a new instance with every invocation



##########
httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartForm.java:
##########
@@ -295,11 +295,11 @@ void testMultipartFormBrowserCompatibleNonASCIIHeaders() 
throws Exception {
         @SuppressWarnings("resource")
         final FormBodyPart p1 = FormBodyPartBuilder.create(
                 "field1",
-                new InputStreamBody(new FileInputStream(tmpfile), s1 + 
".tmp")).build();
+                new InputStreamBody(new FileInputStream(tmpfile), s1 + 
".tmp")).setMode(HttpMultipartMode.LEGACY).build();

Review Comment:
   @arturobernalg Would not it look better as a `#create` parameter?



##########
httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/FormBodyPartBuilder.java:
##########
@@ -96,12 +99,61 @@ public FormBodyPartBuilder setField(final String name, 
final String value) {
         return this;
     }
 
+    /**
+     * Sets the multipart mode for this builder, determining how filenames are 
encoded in the
+     * {@code Content-Disposition} header. The mode affects whether the {@code 
filename*} parameter
+     * is included for non-ISO-8859-1 filenames.
+     * <p>
+     * In {@link HttpMultipartMode#LEGACY} mode, only the {@code filename} 
parameter is included
+     * with the raw value, mimicking pre-RFC 7578 behavior. In {@link 
HttpMultipartMode#STRICT}
+     * or {@link HttpMultipartMode#EXTENDED} modes, the {@code filename*} 
parameter is added
+     * with RFC 5987 encoding for filenames containing non-ISO-8859-1 
characters.
+     * </p>
+     * <p>
+     * If {@code mode} is {@code null}, it defaults to {@link 
HttpMultipartMode#STRICT}.
+     * </p>
+     *
+     * @param mode the {@link HttpMultipartMode} to use, or {@code null} for 
default behavior
+     * @return this builder instance for method chaining
+     * @since 5.5
+     */
+    public FormBodyPartBuilder setMode(final HttpMultipartMode mode) {

Review Comment:
   @arturobernalg Would not it be better to pass this parameter at the 
construction time to `#create` method?



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