You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "arturobernalg (via GitHub)" <gi...@apache.org> on 2025/02/25 08:02:46 UTC
[PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
arturobernalg opened a new pull request, #618:
URL: https://github.com/apache/httpcomponents-client/pull/618
Implements RFC 6266/5987 filename encoding in multipart/form-data. Adds mode-aware support in FormBodyPartBuilder and HttpRFC7578Multipart, propagates mode via MultipartEntityBuilder, and updates tests. Resolves https://issues.apache.org/jira/browse/HTTPCLIENT-2360.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#issuecomment-3061728798
> Question: Wouldn't it be more standard compliant to render the filename* attribute whenever the string is not US-ASCII encodable? E.g., Embedded Tomcat 10.1 does not properly parse the ISO-8859-1 filename content, but correctly handles filename*. It seems to me that the default ISO-8859-1 assumption, although mentioned in some RFCs, is not consistently true in reality.
RFC 7578 §4.2 is crystal clear:
**NOTE: The encoding method described in RFC 5987, which would add a filename* parameter to the Content-Disposition header field, MUST NOT be used.**
HttpClient 5.5 therefore emits filename* only when the filename contains code points that ISO-8859-1 cannot represent. Everything in the 0x00-0xFF range already has a legal encoding, so adding filename* there would break the letter of the spec and has no real-world upside.
If embedded Tomcat 10.1 stumbles over an ISO-8859-1 filename, that is a Tomcat issue, not a protocol requirement. Use HttpMultipartMode.EXTENDED (percent-encoded names) as a workaround or open a bug with Tomcat, but let’s keep HttpClient on the right side of the standards
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#issuecomment-2690416896
@arturobernalg While you are at it, do you think you could remove the code that randomly generates boundary values with a fixed boundary and update the javadocs to instruct the users to use generate their own boundary values if they want them to be random / non-predictable / 'secure'?
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "surlemur (via GitHub)" <gi...@apache.org>.
surlemur commented on PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#issuecomment-3062325154
Ok. I must admit it's hard to grasp the combination of all those related RFCs plus the different traditions...
The pragmatic solution seems to be to assume ISO-8859-1 as the default on the server side when parsing the filename property. I am using the Apache Commons FileUpload and realized that I can set the header encoding there.
Thanks!
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#issuecomment-2686063832
> @arturobernalg Looks good. Some minor suggestions.
HI @ok2c fair enough. I’ve made the changes as recommended. Please take another pass.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
ok2c commented on code in PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#discussion_r1973659740
##########
httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/FormBodyPartBuilder.java:
##########
@@ -47,22 +50,52 @@ public class FormBodyPartBuilder {
private ContentBody body;
private final Header header;
+ /**
+ * The multipart mode determining how filenames are encoded in the {@code Content-Disposition}
+ * header, defaults to {@link HttpMultipartMode#STRICT}.
+ *
+ * @since 5.5
+ */
+ private HttpMultipartMode mode;
+
+ /**
+ * Reusable encoder for ISO-8859-1 charset checks, improving performance by avoiding
+ * repeated instance creation.
+ */
+ private final CharsetEncoder iso8859_1Encoder = StandardCharsets.ISO_8859_1.newEncoder();
Review Comment:
@arturobernalg I would instantiate this attribute lazily. It does not need to be final. Looks good overall.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg merged PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#discussion_r1973844378
##########
httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/FormBodyPartBuilder.java:
##########
@@ -47,22 +50,52 @@ public class FormBodyPartBuilder {
private ContentBody body;
private final Header header;
+ /**
+ * The multipart mode determining how filenames are encoded in the {@code Content-Disposition}
+ * header, defaults to {@link HttpMultipartMode#STRICT}.
+ *
+ * @since 5.5
+ */
+ private HttpMultipartMode mode;
+
+ /**
+ * Reusable encoder for ISO-8859-1 charset checks, improving performance by avoiding
+ * repeated instance creation.
+ */
+ private final CharsetEncoder iso8859_1Encoder = StandardCharsets.ISO_8859_1.newEncoder();
Review Comment:
@ok2c changed.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "ok2c (via GitHub)" <gi...@apache.org>.
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-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#issuecomment-2690478508
> @arturobernalg While you are at it, do you think you could remove the code that randomly generates boundary values with a fixed boundary and update the javadocs to instruct the users to use generate their own boundary values if they want them to be random / non-predictable / 'secure'?
@ok2c sure. I'll do
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "surlemur (via GitHub)" <gi...@apache.org>.
surlemur commented on PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#issuecomment-3061557608
Question: Wouldn't it be more standard compliant to render the filename* attribute whenever the string is not US-ASCII encodable? E.g., Embedded Tomcat 10.1 does not properly parse the ISO-8859-1 filename content, but correctly handles filename*. It seems to me that the default ISO-8859-1 assumption, although mentioned in some RFCs, is not consistently true in reality.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [PR] Enhance filename encoding in multipart/form-data (HTTPCLIENT-2360) [httpcomponents-client]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #618:
URL: https://github.com/apache/httpcomponents-client/pull/618#issuecomment-3062558663
> Ok. I must admit it's hard to grasp the combination of all those related RFCs plus the different traditions...
>
> The pragmatic solution seems to be to assume ISO-8859-1 as the default on the server side when parsing the filename property. I am using the Apache Commons FileUpload and realized that I can set the header encoding there.
>
> Thanks!
@surlemur ,
Correct. RFC 7578 treats the raw octets of a filename parameter as ISO-8859-1 unless they are percent-encoded or a filename*= override is present (see the NOTE in §4.2).
Glad it’s sorted.
--
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@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org