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 2024/09/22 19:07:06 UTC
[PR] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
arturobernalg opened a new pull request, #483:
URL: https://github.com/apache/httpcomponents-core/pull/483
This PR is an attempt to mitigate the issues highlighted in [HTTPCLIENT-2159,](https://issues.apache.org/jira/browse/HTTPCLIENT-2159). The root problem stems from how we are incorrectly handling certain content types by attaching charset parameters when they shouldn't have one. This update adjusts the way the Content-Type header handles specific cases, ensuring compliance with relevant specifications.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#issuecomment-2371320422
> > > I think we should use a builder for ContenType instead of piling on constructors or factory methods.
> >
> >
> > HI @garydgregory That seems like a massive change. Maybe we can consider a full refactor of the class in another ticket? I'm not sure if it's necessary right now compared to the charset ContentType parameter handling.
>
> Hi @arturobernalg Whatever the scale of the change it kind of feels like the right time to do it IMO _because_ we are taking of new factory methods/construction.
>
> I do take your point that it could be done in a separate PR _if_ that PR happens first. If we agree on implementing a builder, I can volunteer to do it unless you'd rather do it.
HI @garydgregory
If we agree on implementing a builder, I will do it.
Thank you.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1773395960
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -67,105 +68,125 @@ public final class ContentType implements Serializable {
*/
private static final String CHARSET = "charset";
+ /**
+ * Flag indicating whether the charset is implicit.
+ * <p>
+ * When {@code implicitCharset} is {@code true}, the charset will not be explicitly
+ * included in the string representation of this {@link ContentType} (i.e., in the {@code toString} method),
+ * unless it is required for the given MIME type.
+ * If {@code implicitCharset} is {@code false}, the charset will always be included in the string representation,
+ * unless the MIME type explicitly disallows charset parameters (e.g., certain binary or multipart types).
+ * </p>
+ * <p>
+ * This flag is essential for proper handling of content types where the charset is either implied by the specification
+ * (e.g., JSON is always UTF-8) or where including a charset is not meaningful (e.g., binary types like
+ * {@code application/octet-stream}).
+ * </p>
+ *
+ * @since 5.5
+ */
+ private final boolean implicitCharset;
+
+
// constants
public static final ContentType APPLICATION_ATOM_XML = create(
- "application/atom+xml", StandardCharsets.UTF_8);
+ "application/atom+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_FORM_URLENCODED = create(
- "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1);
+ "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1, true);
public static final ContentType APPLICATION_JSON = create(
- "application/json", StandardCharsets.UTF_8);
+ "application/json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/x-ndjson}.
* @since 5.1
*/
public static final ContentType APPLICATION_NDJSON = create(
- "application/x-ndjson", StandardCharsets.UTF_8);
+ "application/x-ndjson", StandardCharsets.UTF_8, true);
public static final ContentType APPLICATION_OCTET_STREAM = create(
- "application/octet-stream", (Charset) null);
+ "application/octet-stream", (Charset) null, true);
/**
* Public constant media type for {@code application/pdf}.
* @since 5.1
*/
public static final ContentType APPLICATION_PDF = create(
- "application/pdf", StandardCharsets.UTF_8);
+ "application/pdf", (Charset) null, true);
public static final ContentType APPLICATION_SOAP_XML = create(
- "application/soap+xml", StandardCharsets.UTF_8);
+ "application/soap+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_SVG_XML = create(
- "application/svg+xml", StandardCharsets.UTF_8);
+ "application/svg+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XHTML_XML = create(
- "application/xhtml+xml", StandardCharsets.UTF_8);
+ "application/xhtml+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XML = create(
- "application/xml", StandardCharsets.UTF_8);
+ "application/xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/problem+json}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.1">Problem Details for HTTP APIs, 6.1. application/problem+json</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_JSON = create(
- "application/problem+json", StandardCharsets.UTF_8);
+ "application/problem+json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/problem+xml}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.2">Problem Details for HTTP APIs, 6.2. application/problem+xml</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_XML = create(
- "application/problem+xml", StandardCharsets.UTF_8);
+ "application/problem+xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/rss+xml}.
* @since 5.1
*/
public static final ContentType APPLICATION_RSS_XML = create(
- "application/rss+xml", StandardCharsets.UTF_8);
+ "application/rss+xml", StandardCharsets.UTF_8, false);
public static final ContentType IMAGE_BMP = create(
- "image/bmp");
+ "image/bmp", (Charset) null, true);
public static final ContentType IMAGE_GIF = create(
- "image/gif");
+ "image/gif", (Charset) null, true);
public static final ContentType IMAGE_JPEG = create(
- "image/jpeg");
+ "image/jpeg", (Charset) null, true);
public static final ContentType IMAGE_PNG = create(
- "image/png");
+ "image/png", (Charset) null, true);
public static final ContentType IMAGE_SVG = create(
- "image/svg+xml");
+ "image/svg+xml", (Charset) null, false);
public static final ContentType IMAGE_TIFF = create(
- "image/tiff");
+ "image/tiff", (Charset) null, true);
public static final ContentType IMAGE_WEBP = create(
- "image/webp");
+ "image/webp", (Charset) null, true);
public static final ContentType MULTIPART_FORM_DATA = create(
- "multipart/form-data", StandardCharsets.ISO_8859_1);
+ "multipart/form-data", StandardCharsets.ISO_8859_1, true);
/**
* Public constant media type for {@code multipart/mixed}.
* @since 5.1
*/
public static final ContentType MULTIPART_MIXED = create(
- "multipart/mixed", StandardCharsets.ISO_8859_1);
+ "multipart/mixed", StandardCharsets.ISO_8859_1, true);
/**
* Public constant media type for {@code multipart/related}.
* @since 5.1
*/
public static final ContentType MULTIPART_RELATED = create(
- "multipart/related", StandardCharsets.ISO_8859_1);
+ "multipart/related", StandardCharsets.ISO_8859_1, true);
public static final ContentType TEXT_HTML = create(
- "text/html", StandardCharsets.UTF_8);
+ "text/html", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code text/markdown}.
* @since 5.1
*/
public static final ContentType TEXT_MARKDOWN = create(
- "text/markdown", StandardCharsets.UTF_8);
+ "text/markdown", StandardCharsets.UTF_8, false);
public static final ContentType TEXT_PLAIN = create(
- "text/plain", StandardCharsets.UTF_8);
+ "text/plain", StandardCharsets.UTF_8, true);
Review Comment:
I would really make the charset here explicit, IMHO. No implications.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1771184701
##########
httpcore5/src/test/java/org/apache/hc/core5/http/TestContentType.java:
##########
@@ -183,4 +183,143 @@ void testWithParams() throws Exception {
Assertions.assertEquals("text/blah; charset=ISO-8859-1; p=blah", contentType.toString());
}
+
+ @Test
+ void testImplicitCharsetTrue() {
+ // ContentType with implicitCharset = true
+ final ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, true);
+
+ // Check that the charset is not added to the toString() output
+ Assertions.assertEquals("application/json", contentType.toString());
+ // Check that the charset is still stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testImplicitCharsetFalse() {
+ // ContentType with implicitCharset = false
+ final ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, false);
+
+ // Check that the charset is included in the toString() output
+ Assertions.assertEquals("application/json; charset=UTF-8", contentType.toString());
+ // Check that the charset is correctly stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testImplicitCharsetForTextPlain() {
+ // ContentType for text/plain with implicitCharset = true
+ final ContentType contentType = ContentType.create("text/plain", StandardCharsets.UTF_8, true);
+
+ // Check that the charset is not included in the toString() output
+ Assertions.assertEquals("text/plain", contentType.toString());
+ // Check that the charset is correctly stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testNoCharsetWithImplicitCharset() {
+ // ContentType without a charset, implicitCharset = true
+ final ContentType contentType = ContentType.create("application/octet-stream", null, true);
Review Comment:
This test does not make sense to me since the last param does not matter when no charset has been provided no? The behavior should be undefined if something string-like has been passed, no?
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#issuecomment-2369274855
> This PR does not put the description on the MIME types into consideration pointed out in HTTPCLIENT-2159.
HI @michael-o
Please do another pass. I think we’ve covered all the cases now.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#issuecomment-2369276568
> > This PR does not put the description on the MIME types into consideration pointed out in HTTPCLIENT-2159.
>
> HI @michael-o Please do another pass. I think we’ve covered all the cases now.
Will do tomorrow with a sober mind.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1772056433
##########
httpcore5/src/test/java/org/apache/hc/core5/http/TestContentType.java:
##########
@@ -183,4 +183,132 @@ void testWithParams() throws Exception {
Assertions.assertEquals("text/blah; charset=ISO-8859-1; p=blah", contentType.toString());
}
+
+ @Test
+ void testImplicitCharsetTrue() {
+ // ContentType with implicitCharset = true
+ final ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, true);
+
+ // Check that the charset is not added to the toString() output
+ Assertions.assertEquals("application/json", contentType.toString());
+ // Check that the charset is still stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testImplicitCharsetFalse() {
+ // ContentType with implicitCharset = false
+ final ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, false);
+
+ // Check that the charset is included in the toString() output
+ Assertions.assertEquals("application/json; charset=UTF-8", contentType.toString());
+ // Check that the charset is correctly stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testImplicitCharsetForTextPlain() {
+ // ContentType for text/plain with implicitCharset = true
+ final ContentType contentType = ContentType.create("text/plain", StandardCharsets.UTF_8, true);
+
+ // Check that the charset is not included in the toString() output
+ Assertions.assertEquals("text/plain", contentType.toString());
+ // Check that the charset is correctly stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testWithParamsAndImplicitCharset() {
+ // ContentType with parameters and implicitCharset = true
+ ContentType contentType = ContentType.create("text/plain", StandardCharsets.UTF_8, true)
+ .withParameters(
+ new BasicNameValuePair("p", "this"),
+ new BasicNameValuePair("p", "that"));
+
+ // Check that the last "p" parameter overwrites the first one
+ // ImplicitCharset is true, so charset should not be included
+ Assertions.assertEquals("text/plain; p=that", contentType.toString());
+
+ // Verify that charset is still available in the object
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+
+ // Test with implicitCharset = false
+ contentType = ContentType.create("text/plain", StandardCharsets.UTF_8, false)
+ .withParameters(
+ new BasicNameValuePair("p", "this"),
+ new BasicNameValuePair("p", "that"));
+
+ // Check that the charset is included in the toString() output due to implicitCharset = false
+ Assertions.assertEquals("text/plain; charset=UTF-8; p=that", contentType.toString());
+ }
+
+ @Test
+ void testNoCharsetForSpecificMediaTypes() {
+ // Testing application/octet-stream should not include charset in toString
+ ContentType contentType = ContentType.create("application/octet-stream", StandardCharsets.UTF_8, true);
+ Assertions.assertEquals("application/octet-stream", contentType.toString());
+ Assertions.assertNotNull(contentType.getCharset()); // Ensure charset is set
+
+ // Testing image/jpeg should not include charset in toString
+ contentType = ContentType.create("image/jpeg", StandardCharsets.UTF_8, true);
+ Assertions.assertEquals("image/jpeg", contentType.toString());
+ Assertions.assertNotNull(contentType.getCharset());
+
+ // Testing multipart/form-data should not include charset in toString
+ contentType = ContentType.create("multipart/form-data", StandardCharsets.UTF_8, true);
+ Assertions.assertEquals("multipart/form-data", contentType.toString());
+ Assertions.assertNotNull(contentType.getCharset());
+ }
+
+
+ @Test
+ void testCharsetForOtherMediaTypes() {
+ // Testing application/json should include charset
+ ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, false);
+ Assertions.assertEquals("application/json; charset=UTF-8", contentType.toString());
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+
+ // Testing text/html should include charset
+ contentType = ContentType.create("text/html", StandardCharsets.UTF_8, false);
+ Assertions.assertEquals("text/html; charset=UTF-8", contentType.toString());
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testNoCharsetForBinaryMediaTypes() throws Exception {
+ // Test for application/octet-stream
+ ContentType contentType = ContentType.create("application/octet-stream", null, true);
+ Assertions.assertEquals("application/octet-stream", contentType.toString());
+ Assertions.assertNull(contentType.getCharset());
+
+ // Test for image/jpeg
+ contentType = ContentType.create("image/jpeg", null, true);
+ Assertions.assertEquals("image/jpeg", contentType.toString());
+ Assertions.assertNull(contentType.getCharset());
+ }
+
+ @Test
+ void testFormUrlEncodedWithoutCharset() throws Exception {
+ // Test for application/x-www-form-urlencoded with percent-encoding
+ final ContentType contentType = ContentType.create("application/x-www-form-urlencoded", null, true);
+ Assertions.assertEquals("application/x-www-form-urlencoded", contentType.toString());
+ Assertions.assertNull(contentType.getCharset());
+
+ // Test body encoding example with percent-encoding
+ final String encodedBody = "echotext=TEST%F6TEST";
+ // Simulate HTTP redirect where charset shouldn't change
+ final String redirectedBody = "echotext=TEST%F6TEST";
+ Assertions.assertEquals(encodedBody, redirectedBody); // Ensure body stays the same after redirect
+ }
+
+ @Test
+ void testImplicitCharsetForApplicationJson() throws Exception {
Review Comment:
Isn't this a dup of `testImplicitCharsetTrue`?
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#issuecomment-2369275504
> I think we should use a builder for ContenType instead of piling on constructors or factory methods.
HI @garydgregory
That seems like a massive change. Maybe we can consider a full refactor of the class in another ticket? I'm not sure if it's necessary right now compared to the charset ContentType parameter handling.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1773414411
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -67,105 +68,125 @@ public final class ContentType implements Serializable {
*/
private static final String CHARSET = "charset";
+ /**
+ * Flag indicating whether the charset is implicit.
+ * <p>
+ * When {@code implicitCharset} is {@code true}, the charset will not be explicitly
+ * included in the string representation of this {@link ContentType} (i.e., in the {@code toString} method),
+ * unless it is required for the given MIME type.
+ * If {@code implicitCharset} is {@code false}, the charset will always be included in the string representation,
+ * unless the MIME type explicitly disallows charset parameters (e.g., certain binary or multipart types).
+ * </p>
+ * <p>
+ * This flag is essential for proper handling of content types where the charset is either implied by the specification
+ * (e.g., JSON is always UTF-8) or where including a charset is not meaningful (e.g., binary types like
+ * {@code application/octet-stream}).
+ * </p>
+ *
+ * @since 5.5
+ */
+ private final boolean implicitCharset;
+
+
// constants
public static final ContentType APPLICATION_ATOM_XML = create(
- "application/atom+xml", StandardCharsets.UTF_8);
+ "application/atom+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_FORM_URLENCODED = create(
- "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1);
+ "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1, true);
public static final ContentType APPLICATION_JSON = create(
- "application/json", StandardCharsets.UTF_8);
+ "application/json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/x-ndjson}.
* @since 5.1
*/
public static final ContentType APPLICATION_NDJSON = create(
- "application/x-ndjson", StandardCharsets.UTF_8);
+ "application/x-ndjson", StandardCharsets.UTF_8, true);
public static final ContentType APPLICATION_OCTET_STREAM = create(
- "application/octet-stream", (Charset) null);
+ "application/octet-stream", (Charset) null, true);
/**
* Public constant media type for {@code application/pdf}.
* @since 5.1
*/
public static final ContentType APPLICATION_PDF = create(
- "application/pdf", StandardCharsets.UTF_8);
+ "application/pdf", (Charset) null, true);
public static final ContentType APPLICATION_SOAP_XML = create(
- "application/soap+xml", StandardCharsets.UTF_8);
+ "application/soap+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_SVG_XML = create(
- "application/svg+xml", StandardCharsets.UTF_8);
+ "application/svg+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XHTML_XML = create(
- "application/xhtml+xml", StandardCharsets.UTF_8);
+ "application/xhtml+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XML = create(
- "application/xml", StandardCharsets.UTF_8);
+ "application/xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/problem+json}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.1">Problem Details for HTTP APIs, 6.1. application/problem+json</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_JSON = create(
- "application/problem+json", StandardCharsets.UTF_8);
+ "application/problem+json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/problem+xml}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.2">Problem Details for HTTP APIs, 6.2. application/problem+xml</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_XML = create(
- "application/problem+xml", StandardCharsets.UTF_8);
+ "application/problem+xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/rss+xml}.
* @since 5.1
*/
public static final ContentType APPLICATION_RSS_XML = create(
- "application/rss+xml", StandardCharsets.UTF_8);
+ "application/rss+xml", StandardCharsets.UTF_8, false);
public static final ContentType IMAGE_BMP = create(
- "image/bmp");
+ "image/bmp", (Charset) null, true);
public static final ContentType IMAGE_GIF = create(
- "image/gif");
+ "image/gif", (Charset) null, true);
public static final ContentType IMAGE_JPEG = create(
- "image/jpeg");
+ "image/jpeg", (Charset) null, true);
public static final ContentType IMAGE_PNG = create(
- "image/png");
+ "image/png", (Charset) null, true);
public static final ContentType IMAGE_SVG = create(
- "image/svg+xml");
+ "image/svg+xml", (Charset) null, false);
public static final ContentType IMAGE_TIFF = create(
- "image/tiff");
+ "image/tiff", (Charset) null, true);
public static final ContentType IMAGE_WEBP = create(
- "image/webp");
+ "image/webp", (Charset) null, true);
public static final ContentType MULTIPART_FORM_DATA = create(
- "multipart/form-data", StandardCharsets.ISO_8859_1);
+ "multipart/form-data", StandardCharsets.ISO_8859_1, true);
/**
* Public constant media type for {@code multipart/mixed}.
* @since 5.1
*/
public static final ContentType MULTIPART_MIXED = create(
- "multipart/mixed", StandardCharsets.ISO_8859_1);
+ "multipart/mixed", StandardCharsets.ISO_8859_1, true);
/**
* Public constant media type for {@code multipart/related}.
* @since 5.1
*/
public static final ContentType MULTIPART_RELATED = create(
- "multipart/related", StandardCharsets.ISO_8859_1);
+ "multipart/related", StandardCharsets.ISO_8859_1, true);
public static final ContentType TEXT_HTML = create(
- "text/html", StandardCharsets.UTF_8);
+ "text/html", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code text/markdown}.
* @since 5.1
*/
public static final ContentType TEXT_MARKDOWN = create(
- "text/markdown", StandardCharsets.UTF_8);
+ "text/markdown", StandardCharsets.UTF_8, false);
public static final ContentType TEXT_PLAIN = create(
- "text/plain", StandardCharsets.UTF_8);
+ "text/plain", StandardCharsets.UTF_8, true);
Review Comment:
fair enough
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1772051216
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -67,105 +68,125 @@ public final class ContentType implements Serializable {
*/
private static final String CHARSET = "charset";
+ /**
+ * Flag indicating whether the charset is implicit.
+ * <p>
+ * When {@code implicitCharset} is {@code true}, the charset will not be explicitly
+ * included in the string representation of this {@link ContentType} (i.e., in the {@code toString} method),
+ * unless it is required for the given MIME type.
+ * If {@code implicitCharset} is {@code false}, the charset will always be included in the string representation,
+ * unless the MIME type explicitly disallows charset parameters (e.g., certain binary or multipart types).
+ * </p>
+ * <p>
+ * This flag is essential for proper handling of content types where the charset is either implied by the specification
+ * (e.g., JSON is always UTF-8) or where including a charset is not meaningful (e.g., binary types like
+ * {@code application/octet-stream}).
+ * </p>
+ *
+ * @since 5.5
+ */
+ private final boolean implicitCharset;
+
+
// constants
public static final ContentType APPLICATION_ATOM_XML = create(
- "application/atom+xml", StandardCharsets.UTF_8);
+ "application/atom+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_FORM_URLENCODED = create(
- "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1);
+ "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1, true);
public static final ContentType APPLICATION_JSON = create(
- "application/json", StandardCharsets.UTF_8);
+ "application/json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/x-ndjson}.
* @since 5.1
*/
public static final ContentType APPLICATION_NDJSON = create(
- "application/x-ndjson", StandardCharsets.UTF_8);
+ "application/x-ndjson", StandardCharsets.UTF_8, true);
public static final ContentType APPLICATION_OCTET_STREAM = create(
- "application/octet-stream", (Charset) null);
+ "application/octet-stream", (Charset) null, true);
/**
* Public constant media type for {@code application/pdf}.
* @since 5.1
*/
public static final ContentType APPLICATION_PDF = create(
- "application/pdf", StandardCharsets.UTF_8);
+ "application/pdf", (Charset) null, true);
public static final ContentType APPLICATION_SOAP_XML = create(
- "application/soap+xml", StandardCharsets.UTF_8);
+ "application/soap+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_SVG_XML = create(
- "application/svg+xml", StandardCharsets.UTF_8);
+ "application/svg+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XHTML_XML = create(
- "application/xhtml+xml", StandardCharsets.UTF_8);
+ "application/xhtml+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XML = create(
- "application/xml", StandardCharsets.UTF_8);
+ "application/xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/problem+json}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.1">Problem Details for HTTP APIs, 6.1. application/problem+json</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_JSON = create(
- "application/problem+json", StandardCharsets.UTF_8);
+ "application/problem+json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/problem+xml}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.2">Problem Details for HTTP APIs, 6.2. application/problem+xml</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_XML = create(
- "application/problem+xml", StandardCharsets.UTF_8);
+ "application/problem+xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/rss+xml}.
* @since 5.1
*/
public static final ContentType APPLICATION_RSS_XML = create(
- "application/rss+xml", StandardCharsets.UTF_8);
+ "application/rss+xml", StandardCharsets.UTF_8, false);
public static final ContentType IMAGE_BMP = create(
- "image/bmp");
+ "image/bmp", (Charset) null, true);
public static final ContentType IMAGE_GIF = create(
- "image/gif");
+ "image/gif", (Charset) null, true);
public static final ContentType IMAGE_JPEG = create(
- "image/jpeg");
+ "image/jpeg", (Charset) null, true);
public static final ContentType IMAGE_PNG = create(
- "image/png");
+ "image/png", (Charset) null, true);
public static final ContentType IMAGE_SVG = create(
- "image/svg+xml");
+ "image/svg+xml", (Charset) null, false);
public static final ContentType IMAGE_TIFF = create(
- "image/tiff");
+ "image/tiff", (Charset) null, true);
public static final ContentType IMAGE_WEBP = create(
- "image/webp");
+ "image/webp", (Charset) null, true);
public static final ContentType MULTIPART_FORM_DATA = create(
- "multipart/form-data", StandardCharsets.ISO_8859_1);
+ "multipart/form-data", StandardCharsets.ISO_8859_1, true);
/**
* Public constant media type for {@code multipart/mixed}.
* @since 5.1
*/
public static final ContentType MULTIPART_MIXED = create(
- "multipart/mixed", StandardCharsets.ISO_8859_1);
+ "multipart/mixed", StandardCharsets.ISO_8859_1, true);
/**
* Public constant media type for {@code multipart/related}.
* @since 5.1
*/
public static final ContentType MULTIPART_RELATED = create(
- "multipart/related", StandardCharsets.ISO_8859_1);
+ "multipart/related", StandardCharsets.ISO_8859_1, true);
public static final ContentType TEXT_HTML = create(
- "text/html", StandardCharsets.UTF_8);
+ "text/html", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code text/markdown}.
* @since 5.1
*/
public static final ContentType TEXT_MARKDOWN = create(
- "text/markdown", StandardCharsets.UTF_8);
+ "text/markdown", StandardCharsets.UTF_8, false);
public static final ContentType TEXT_PLAIN = create(
- "text/plain", StandardCharsets.UTF_8);
+ "text/plain", StandardCharsets.UTF_8, true);
Review Comment:
Are you certain about this or is this our asssumption these days?
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1772049067
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -67,105 +68,125 @@ public final class ContentType implements Serializable {
*/
private static final String CHARSET = "charset";
+ /**
+ * Flag indicating whether the charset is implicit.
+ * <p>
+ * When {@code implicitCharset} is {@code true}, the charset will not be explicitly
+ * included in the string representation of this {@link ContentType} (i.e., in the {@code toString} method),
+ * unless it is required for the given MIME type.
+ * If {@code implicitCharset} is {@code false}, the charset will always be included in the string representation,
+ * unless the MIME type explicitly disallows charset parameters (e.g., certain binary or multipart types).
+ * </p>
+ * <p>
+ * This flag is essential for proper handling of content types where the charset is either implied by the specification
+ * (e.g., JSON is always UTF-8) or where including a charset is not meaningful (e.g., binary types like
+ * {@code application/octet-stream}).
+ * </p>
+ *
+ * @since 5.5
+ */
+ private final boolean implicitCharset;
+
+
// constants
public static final ContentType APPLICATION_ATOM_XML = create(
- "application/atom+xml", StandardCharsets.UTF_8);
+ "application/atom+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_FORM_URLENCODED = create(
- "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1);
+ "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1, true);
public static final ContentType APPLICATION_JSON = create(
- "application/json", StandardCharsets.UTF_8);
+ "application/json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/x-ndjson}.
* @since 5.1
*/
public static final ContentType APPLICATION_NDJSON = create(
- "application/x-ndjson", StandardCharsets.UTF_8);
+ "application/x-ndjson", StandardCharsets.UTF_8, true);
public static final ContentType APPLICATION_OCTET_STREAM = create(
- "application/octet-stream", (Charset) null);
+ "application/octet-stream", (Charset) null, true);
/**
* Public constant media type for {@code application/pdf}.
* @since 5.1
*/
public static final ContentType APPLICATION_PDF = create(
- "application/pdf", StandardCharsets.UTF_8);
+ "application/pdf", (Charset) null, true);
public static final ContentType APPLICATION_SOAP_XML = create(
- "application/soap+xml", StandardCharsets.UTF_8);
+ "application/soap+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_SVG_XML = create(
- "application/svg+xml", StandardCharsets.UTF_8);
+ "application/svg+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XHTML_XML = create(
- "application/xhtml+xml", StandardCharsets.UTF_8);
+ "application/xhtml+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XML = create(
- "application/xml", StandardCharsets.UTF_8);
+ "application/xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/problem+json}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.1">Problem Details for HTTP APIs, 6.1. application/problem+json</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_JSON = create(
- "application/problem+json", StandardCharsets.UTF_8);
+ "application/problem+json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/problem+xml}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.2">Problem Details for HTTP APIs, 6.2. application/problem+xml</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_XML = create(
- "application/problem+xml", StandardCharsets.UTF_8);
+ "application/problem+xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/rss+xml}.
* @since 5.1
*/
public static final ContentType APPLICATION_RSS_XML = create(
- "application/rss+xml", StandardCharsets.UTF_8);
+ "application/rss+xml", StandardCharsets.UTF_8, false);
public static final ContentType IMAGE_BMP = create(
- "image/bmp");
+ "image/bmp", (Charset) null, true);
public static final ContentType IMAGE_GIF = create(
- "image/gif");
+ "image/gif", (Charset) null, true);
public static final ContentType IMAGE_JPEG = create(
- "image/jpeg");
+ "image/jpeg", (Charset) null, true);
public static final ContentType IMAGE_PNG = create(
- "image/png");
+ "image/png", (Charset) null, true);
public static final ContentType IMAGE_SVG = create(
- "image/svg+xml");
+ "image/svg+xml", (Charset) null, false);
Review Comment:
Well, it is still XML, means text...
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1772057511
##########
httpcore5/src/test/java/org/apache/hc/core5/http/TestContentType.java:
##########
@@ -183,4 +183,132 @@ void testWithParams() throws Exception {
Assertions.assertEquals("text/blah; charset=ISO-8859-1; p=blah", contentType.toString());
}
+
+ @Test
+ void testImplicitCharsetTrue() {
+ // ContentType with implicitCharset = true
+ final ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, true);
+
+ // Check that the charset is not added to the toString() output
+ Assertions.assertEquals("application/json", contentType.toString());
+ // Check that the charset is still stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testImplicitCharsetFalse() {
+ // ContentType with implicitCharset = false
+ final ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, false);
+
+ // Check that the charset is included in the toString() output
+ Assertions.assertEquals("application/json; charset=UTF-8", contentType.toString());
+ // Check that the charset is correctly stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testImplicitCharsetForTextPlain() {
+ // ContentType for text/plain with implicitCharset = true
+ final ContentType contentType = ContentType.create("text/plain", StandardCharsets.UTF_8, true);
+
+ // Check that the charset is not included in the toString() output
+ Assertions.assertEquals("text/plain", contentType.toString());
+ // Check that the charset is correctly stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testWithParamsAndImplicitCharset() {
+ // ContentType with parameters and implicitCharset = true
+ ContentType contentType = ContentType.create("text/plain", StandardCharsets.UTF_8, true)
+ .withParameters(
+ new BasicNameValuePair("p", "this"),
+ new BasicNameValuePair("p", "that"));
+
+ // Check that the last "p" parameter overwrites the first one
+ // ImplicitCharset is true, so charset should not be included
+ Assertions.assertEquals("text/plain; p=that", contentType.toString());
+
+ // Verify that charset is still available in the object
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+
+ // Test with implicitCharset = false
+ contentType = ContentType.create("text/plain", StandardCharsets.UTF_8, false)
+ .withParameters(
+ new BasicNameValuePair("p", "this"),
+ new BasicNameValuePair("p", "that"));
+
+ // Check that the charset is included in the toString() output due to implicitCharset = false
+ Assertions.assertEquals("text/plain; charset=UTF-8; p=that", contentType.toString());
+ }
+
+ @Test
+ void testNoCharsetForSpecificMediaTypes() {
+ // Testing application/octet-stream should not include charset in toString
+ ContentType contentType = ContentType.create("application/octet-stream", StandardCharsets.UTF_8, true);
+ Assertions.assertEquals("application/octet-stream", contentType.toString());
+ Assertions.assertNotNull(contentType.getCharset()); // Ensure charset is set
+
+ // Testing image/jpeg should not include charset in toString
+ contentType = ContentType.create("image/jpeg", StandardCharsets.UTF_8, true);
+ Assertions.assertEquals("image/jpeg", contentType.toString());
+ Assertions.assertNotNull(contentType.getCharset());
+
+ // Testing multipart/form-data should not include charset in toString
+ contentType = ContentType.create("multipart/form-data", StandardCharsets.UTF_8, true);
+ Assertions.assertEquals("multipart/form-data", contentType.toString());
+ Assertions.assertNotNull(contentType.getCharset());
+ }
+
+
+ @Test
+ void testCharsetForOtherMediaTypes() {
+ // Testing application/json should include charset
+ ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, false);
+ Assertions.assertEquals("application/json; charset=UTF-8", contentType.toString());
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+
+ // Testing text/html should include charset
+ contentType = ContentType.create("text/html", StandardCharsets.UTF_8, false);
+ Assertions.assertEquals("text/html; charset=UTF-8", contentType.toString());
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
+
+ @Test
+ void testNoCharsetForBinaryMediaTypes() throws Exception {
+ // Test for application/octet-stream
+ ContentType contentType = ContentType.create("application/octet-stream", null, true);
+ Assertions.assertEquals("application/octet-stream", contentType.toString());
+ Assertions.assertNull(contentType.getCharset());
+
+ // Test for image/jpeg
+ contentType = ContentType.create("image/jpeg", null, true);
+ Assertions.assertEquals("image/jpeg", contentType.toString());
+ Assertions.assertNull(contentType.getCharset());
+ }
+
+ @Test
+ void testFormUrlEncodedWithoutCharset() throws Exception {
+ // Test for application/x-www-form-urlencoded with percent-encoding
+ final ContentType contentType = ContentType.create("application/x-www-form-urlencoded", null, true);
+ Assertions.assertEquals("application/x-www-form-urlencoded", contentType.toString());
+ Assertions.assertNull(contentType.getCharset());
+
+ // Test body encoding example with percent-encoding
+ final String encodedBody = "echotext=TEST%F6TEST";
+ // Simulate HTTP redirect where charset shouldn't change
+ final String redirectedBody = "echotext=TEST%F6TEST";
+ Assertions.assertEquals(encodedBody, redirectedBody); // Ensure body stays the same after redirect
+ }
+
+ @Test
+ void testImplicitCharsetForApplicationJson() throws Exception {
+ // ContentType for application/json with implicitCharset = true
+ final ContentType contentType = ContentType.create("application/json", StandardCharsets.UTF_8, true);
+
+ // Check that the charset is not added to the toString() output
+ Assertions.assertEquals("application/json", contentType.toString());
+ // Check that the charset is still stored
+ Assertions.assertEquals(StandardCharsets.UTF_8, contentType.getCharset());
+ }
Review Comment:
What about `foo/bar`, null, false`? With a null charset the last param should not matter.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1772063397
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -326,7 +432,9 @@ public static ContentType create(final String mimeType, final Charset charset) {
* @param mimeType MIME type. It may not be {@code null} or empty. It may not contain
* characters {@code <">, <;>, <,>} reserved by the HTTP specification.
* @return content type
+ * @deprecated Use {@link #create(String, Charset, boolean)} with an explicit charset or null, and specify if the charset is implicit.
*/
+ @Deprecated
Review Comment:
Why deprecate a short-hand ctor?
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1771188233
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -214,6 +236,18 @@ public final class ContentType implements Serializable {
CONTENT_TYPE_MAP = Collections.unmodifiableMap(map);
}
+ // List of media types that should not include charset
+ private static final List<String> NO_CHARSET_TYPES = Collections.unmodifiableList(Arrays.asList(
Review Comment:
I wouldn't maintain such a like because it can never be exhaustive. We have some predefines and they should carry metadata. If a client creates a custom content type it is his obligation to provide this information, no?
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
Review Comment:
PDF is not text. It is a binary format. The charset should be dropped.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1773392198
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -67,105 +68,125 @@ public final class ContentType implements Serializable {
*/
private static final String CHARSET = "charset";
+ /**
+ * Flag indicating whether the charset is implicit.
+ * <p>
+ * When {@code implicitCharset} is {@code true}, the charset will not be explicitly
+ * included in the string representation of this {@link ContentType} (i.e., in the {@code toString} method),
+ * unless it is required for the given MIME type.
+ * If {@code implicitCharset} is {@code false}, the charset will always be included in the string representation,
+ * unless the MIME type explicitly disallows charset parameters (e.g., certain binary or multipart types).
+ * </p>
+ * <p>
+ * This flag is essential for proper handling of content types where the charset is either implied by the specification
+ * (e.g., JSON is always UTF-8) or where including a charset is not meaningful (e.g., binary types like
+ * {@code application/octet-stream}).
+ * </p>
+ *
+ * @since 5.5
+ */
+ private final boolean implicitCharset;
+
+
// constants
public static final ContentType APPLICATION_ATOM_XML = create(
- "application/atom+xml", StandardCharsets.UTF_8);
+ "application/atom+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_FORM_URLENCODED = create(
- "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1);
+ "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1, true);
public static final ContentType APPLICATION_JSON = create(
- "application/json", StandardCharsets.UTF_8);
+ "application/json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/x-ndjson}.
* @since 5.1
*/
public static final ContentType APPLICATION_NDJSON = create(
- "application/x-ndjson", StandardCharsets.UTF_8);
+ "application/x-ndjson", StandardCharsets.UTF_8, true);
public static final ContentType APPLICATION_OCTET_STREAM = create(
- "application/octet-stream", (Charset) null);
+ "application/octet-stream", (Charset) null, true);
/**
* Public constant media type for {@code application/pdf}.
* @since 5.1
*/
public static final ContentType APPLICATION_PDF = create(
- "application/pdf", StandardCharsets.UTF_8);
+ "application/pdf", (Charset) null, true);
public static final ContentType APPLICATION_SOAP_XML = create(
- "application/soap+xml", StandardCharsets.UTF_8);
+ "application/soap+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_SVG_XML = create(
- "application/svg+xml", StandardCharsets.UTF_8);
+ "application/svg+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XHTML_XML = create(
- "application/xhtml+xml", StandardCharsets.UTF_8);
+ "application/xhtml+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XML = create(
- "application/xml", StandardCharsets.UTF_8);
+ "application/xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/problem+json}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.1">Problem Details for HTTP APIs, 6.1. application/problem+json</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_JSON = create(
- "application/problem+json", StandardCharsets.UTF_8);
+ "application/problem+json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/problem+xml}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.2">Problem Details for HTTP APIs, 6.2. application/problem+xml</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_XML = create(
- "application/problem+xml", StandardCharsets.UTF_8);
+ "application/problem+xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/rss+xml}.
* @since 5.1
*/
public static final ContentType APPLICATION_RSS_XML = create(
- "application/rss+xml", StandardCharsets.UTF_8);
+ "application/rss+xml", StandardCharsets.UTF_8, false);
public static final ContentType IMAGE_BMP = create(
- "image/bmp");
+ "image/bmp", (Charset) null, true);
public static final ContentType IMAGE_GIF = create(
- "image/gif");
+ "image/gif", (Charset) null, true);
public static final ContentType IMAGE_JPEG = create(
- "image/jpeg");
+ "image/jpeg", (Charset) null, true);
public static final ContentType IMAGE_PNG = create(
- "image/png");
+ "image/png", (Charset) null, true);
public static final ContentType IMAGE_SVG = create(
- "image/svg+xml");
+ "image/svg+xml", (Charset) null, false);
Review Comment:
The same argument would apply to any XML type document: leave charset out.
--
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] HTTPCLIENT-2159: Invalid handling of charset content type parameter [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#issuecomment-2380677407
> Code is fine for me now, but the title is wrong. The actual report does not relate to any redirects at all. Please change and we are good to merge.
done
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1773414963
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -67,105 +68,125 @@ public final class ContentType implements Serializable {
*/
private static final String CHARSET = "charset";
+ /**
+ * Flag indicating whether the charset is implicit.
+ * <p>
+ * When {@code implicitCharset} is {@code true}, the charset will not be explicitly
+ * included in the string representation of this {@link ContentType} (i.e., in the {@code toString} method),
+ * unless it is required for the given MIME type.
+ * If {@code implicitCharset} is {@code false}, the charset will always be included in the string representation,
+ * unless the MIME type explicitly disallows charset parameters (e.g., certain binary or multipart types).
+ * </p>
+ * <p>
+ * This flag is essential for proper handling of content types where the charset is either implied by the specification
+ * (e.g., JSON is always UTF-8) or where including a charset is not meaningful (e.g., binary types like
+ * {@code application/octet-stream}).
+ * </p>
+ *
+ * @since 5.5
+ */
+ private final boolean implicitCharset;
+
+
// constants
public static final ContentType APPLICATION_ATOM_XML = create(
- "application/atom+xml", StandardCharsets.UTF_8);
+ "application/atom+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_FORM_URLENCODED = create(
- "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1);
+ "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1, true);
public static final ContentType APPLICATION_JSON = create(
- "application/json", StandardCharsets.UTF_8);
+ "application/json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/x-ndjson}.
* @since 5.1
*/
public static final ContentType APPLICATION_NDJSON = create(
- "application/x-ndjson", StandardCharsets.UTF_8);
+ "application/x-ndjson", StandardCharsets.UTF_8, true);
public static final ContentType APPLICATION_OCTET_STREAM = create(
- "application/octet-stream", (Charset) null);
+ "application/octet-stream", (Charset) null, true);
/**
* Public constant media type for {@code application/pdf}.
* @since 5.1
*/
public static final ContentType APPLICATION_PDF = create(
- "application/pdf", StandardCharsets.UTF_8);
+ "application/pdf", (Charset) null, true);
public static final ContentType APPLICATION_SOAP_XML = create(
- "application/soap+xml", StandardCharsets.UTF_8);
+ "application/soap+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_SVG_XML = create(
- "application/svg+xml", StandardCharsets.UTF_8);
+ "application/svg+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XHTML_XML = create(
- "application/xhtml+xml", StandardCharsets.UTF_8);
+ "application/xhtml+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XML = create(
- "application/xml", StandardCharsets.UTF_8);
+ "application/xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/problem+json}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.1">Problem Details for HTTP APIs, 6.1. application/problem+json</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_JSON = create(
- "application/problem+json", StandardCharsets.UTF_8);
+ "application/problem+json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/problem+xml}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.2">Problem Details for HTTP APIs, 6.2. application/problem+xml</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_XML = create(
- "application/problem+xml", StandardCharsets.UTF_8);
+ "application/problem+xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/rss+xml}.
* @since 5.1
*/
public static final ContentType APPLICATION_RSS_XML = create(
- "application/rss+xml", StandardCharsets.UTF_8);
+ "application/rss+xml", StandardCharsets.UTF_8, false);
public static final ContentType IMAGE_BMP = create(
- "image/bmp");
+ "image/bmp", (Charset) null, true);
public static final ContentType IMAGE_GIF = create(
- "image/gif");
+ "image/gif", (Charset) null, true);
public static final ContentType IMAGE_JPEG = create(
- "image/jpeg");
+ "image/jpeg", (Charset) null, true);
public static final ContentType IMAGE_PNG = create(
- "image/png");
+ "image/png", (Charset) null, true);
public static final ContentType IMAGE_SVG = create(
- "image/svg+xml");
+ "image/svg+xml", (Charset) null, false);
Review Comment:
cahnged
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1773349720
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -67,105 +68,125 @@ public final class ContentType implements Serializable {
*/
private static final String CHARSET = "charset";
+ /**
+ * Flag indicating whether the charset is implicit.
+ * <p>
+ * When {@code implicitCharset} is {@code true}, the charset will not be explicitly
+ * included in the string representation of this {@link ContentType} (i.e., in the {@code toString} method),
+ * unless it is required for the given MIME type.
+ * If {@code implicitCharset} is {@code false}, the charset will always be included in the string representation,
+ * unless the MIME type explicitly disallows charset parameters (e.g., certain binary or multipart types).
+ * </p>
+ * <p>
+ * This flag is essential for proper handling of content types where the charset is either implied by the specification
+ * (e.g., JSON is always UTF-8) or where including a charset is not meaningful (e.g., binary types like
+ * {@code application/octet-stream}).
+ * </p>
+ *
+ * @since 5.5
+ */
+ private final boolean implicitCharset;
+
+
// constants
public static final ContentType APPLICATION_ATOM_XML = create(
- "application/atom+xml", StandardCharsets.UTF_8);
+ "application/atom+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_FORM_URLENCODED = create(
- "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1);
+ "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1, true);
public static final ContentType APPLICATION_JSON = create(
- "application/json", StandardCharsets.UTF_8);
+ "application/json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/x-ndjson}.
* @since 5.1
*/
public static final ContentType APPLICATION_NDJSON = create(
- "application/x-ndjson", StandardCharsets.UTF_8);
+ "application/x-ndjson", StandardCharsets.UTF_8, true);
public static final ContentType APPLICATION_OCTET_STREAM = create(
- "application/octet-stream", (Charset) null);
+ "application/octet-stream", (Charset) null, true);
/**
* Public constant media type for {@code application/pdf}.
* @since 5.1
*/
public static final ContentType APPLICATION_PDF = create(
- "application/pdf", StandardCharsets.UTF_8);
+ "application/pdf", (Charset) null, true);
public static final ContentType APPLICATION_SOAP_XML = create(
- "application/soap+xml", StandardCharsets.UTF_8);
+ "application/soap+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_SVG_XML = create(
- "application/svg+xml", StandardCharsets.UTF_8);
+ "application/svg+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XHTML_XML = create(
- "application/xhtml+xml", StandardCharsets.UTF_8);
+ "application/xhtml+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XML = create(
- "application/xml", StandardCharsets.UTF_8);
+ "application/xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/problem+json}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.1">Problem Details for HTTP APIs, 6.1. application/problem+json</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_JSON = create(
- "application/problem+json", StandardCharsets.UTF_8);
+ "application/problem+json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/problem+xml}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.2">Problem Details for HTTP APIs, 6.2. application/problem+xml</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_XML = create(
- "application/problem+xml", StandardCharsets.UTF_8);
+ "application/problem+xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/rss+xml}.
* @since 5.1
*/
public static final ContentType APPLICATION_RSS_XML = create(
- "application/rss+xml", StandardCharsets.UTF_8);
+ "application/rss+xml", StandardCharsets.UTF_8, false);
public static final ContentType IMAGE_BMP = create(
- "image/bmp");
+ "image/bmp", (Charset) null, true);
public static final ContentType IMAGE_GIF = create(
- "image/gif");
+ "image/gif", (Charset) null, true);
public static final ContentType IMAGE_JPEG = create(
- "image/jpeg");
+ "image/jpeg", (Charset) null, true);
public static final ContentType IMAGE_PNG = create(
- "image/png");
+ "image/png", (Charset) null, true);
public static final ContentType IMAGE_SVG = create(
- "image/svg+xml");
+ "image/svg+xml", (Charset) null, false);
Review Comment:
HI @michael-o
I understand that SVG is XML-based, and UTF-8 is commonly used as the default encoding for XML documents. However, leaving the charset as null in this case allows flexibility for other charsets to be specified in the SVG document itself, if needed (e.g., ISO-8859-1). This ensures the ContentType class respects the encoding declared within the SVG document rather than forcing UTF-8 by default. If you believe UTF-8 should be enforced due to its predominance in XML use cases, I can make the 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@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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg commented on code in PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#discussion_r1773363903
##########
httpcore5/src/main/java/org/apache/hc/core5/http/ContentType.java:
##########
@@ -67,105 +68,125 @@ public final class ContentType implements Serializable {
*/
private static final String CHARSET = "charset";
+ /**
+ * Flag indicating whether the charset is implicit.
+ * <p>
+ * When {@code implicitCharset} is {@code true}, the charset will not be explicitly
+ * included in the string representation of this {@link ContentType} (i.e., in the {@code toString} method),
+ * unless it is required for the given MIME type.
+ * If {@code implicitCharset} is {@code false}, the charset will always be included in the string representation,
+ * unless the MIME type explicitly disallows charset parameters (e.g., certain binary or multipart types).
+ * </p>
+ * <p>
+ * This flag is essential for proper handling of content types where the charset is either implied by the specification
+ * (e.g., JSON is always UTF-8) or where including a charset is not meaningful (e.g., binary types like
+ * {@code application/octet-stream}).
+ * </p>
+ *
+ * @since 5.5
+ */
+ private final boolean implicitCharset;
+
+
// constants
public static final ContentType APPLICATION_ATOM_XML = create(
- "application/atom+xml", StandardCharsets.UTF_8);
+ "application/atom+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_FORM_URLENCODED = create(
- "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1);
+ "application/x-www-form-urlencoded", StandardCharsets.ISO_8859_1, true);
public static final ContentType APPLICATION_JSON = create(
- "application/json", StandardCharsets.UTF_8);
+ "application/json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/x-ndjson}.
* @since 5.1
*/
public static final ContentType APPLICATION_NDJSON = create(
- "application/x-ndjson", StandardCharsets.UTF_8);
+ "application/x-ndjson", StandardCharsets.UTF_8, true);
public static final ContentType APPLICATION_OCTET_STREAM = create(
- "application/octet-stream", (Charset) null);
+ "application/octet-stream", (Charset) null, true);
/**
* Public constant media type for {@code application/pdf}.
* @since 5.1
*/
public static final ContentType APPLICATION_PDF = create(
- "application/pdf", StandardCharsets.UTF_8);
+ "application/pdf", (Charset) null, true);
public static final ContentType APPLICATION_SOAP_XML = create(
- "application/soap+xml", StandardCharsets.UTF_8);
+ "application/soap+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_SVG_XML = create(
- "application/svg+xml", StandardCharsets.UTF_8);
+ "application/svg+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XHTML_XML = create(
- "application/xhtml+xml", StandardCharsets.UTF_8);
+ "application/xhtml+xml", StandardCharsets.UTF_8, false);
public static final ContentType APPLICATION_XML = create(
- "application/xml", StandardCharsets.UTF_8);
+ "application/xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/problem+json}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.1">Problem Details for HTTP APIs, 6.1. application/problem+json</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_JSON = create(
- "application/problem+json", StandardCharsets.UTF_8);
+ "application/problem+json", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code application/problem+xml}.
* @see <a href="https://tools.ietf.org/html/rfc7807#section-6.2">Problem Details for HTTP APIs, 6.2. application/problem+xml</a>
* @since 5.1
*/
public static final ContentType APPLICATION_PROBLEM_XML = create(
- "application/problem+xml", StandardCharsets.UTF_8);
+ "application/problem+xml", StandardCharsets.UTF_8, false);
/**
* Public constant media type for {@code application/rss+xml}.
* @since 5.1
*/
public static final ContentType APPLICATION_RSS_XML = create(
- "application/rss+xml", StandardCharsets.UTF_8);
+ "application/rss+xml", StandardCharsets.UTF_8, false);
public static final ContentType IMAGE_BMP = create(
- "image/bmp");
+ "image/bmp", (Charset) null, true);
public static final ContentType IMAGE_GIF = create(
- "image/gif");
+ "image/gif", (Charset) null, true);
public static final ContentType IMAGE_JPEG = create(
- "image/jpeg");
+ "image/jpeg", (Charset) null, true);
public static final ContentType IMAGE_PNG = create(
- "image/png");
+ "image/png", (Charset) null, true);
public static final ContentType IMAGE_SVG = create(
- "image/svg+xml");
+ "image/svg+xml", (Charset) null, false);
public static final ContentType IMAGE_TIFF = create(
- "image/tiff");
+ "image/tiff", (Charset) null, true);
public static final ContentType IMAGE_WEBP = create(
- "image/webp");
+ "image/webp", (Charset) null, true);
public static final ContentType MULTIPART_FORM_DATA = create(
- "multipart/form-data", StandardCharsets.ISO_8859_1);
+ "multipart/form-data", StandardCharsets.ISO_8859_1, true);
/**
* Public constant media type for {@code multipart/mixed}.
* @since 5.1
*/
public static final ContentType MULTIPART_MIXED = create(
- "multipart/mixed", StandardCharsets.ISO_8859_1);
+ "multipart/mixed", StandardCharsets.ISO_8859_1, true);
/**
* Public constant media type for {@code multipart/related}.
* @since 5.1
*/
public static final ContentType MULTIPART_RELATED = create(
- "multipart/related", StandardCharsets.ISO_8859_1);
+ "multipart/related", StandardCharsets.ISO_8859_1, true);
public static final ContentType TEXT_HTML = create(
- "text/html", StandardCharsets.UTF_8);
+ "text/html", StandardCharsets.UTF_8, true);
/**
* Public constant media type for {@code text/markdown}.
* @since 5.1
*/
public static final ContentType TEXT_MARKDOWN = create(
- "text/markdown", StandardCharsets.UTF_8);
+ "text/markdown", StandardCharsets.UTF_8, false);
public static final ContentType TEXT_PLAIN = create(
- "text/plain", StandardCharsets.UTF_8);
+ "text/plain", StandardCharsets.UTF_8, true);
Review Comment:
According to RFC 7231 text/plain without a specified charset does not have a default anymore, and it's up to the media type definition. However, it's widely accepted today that UTF-8 is the most common and preferred encoding, especially with its prominence in web standards and internationalization efforts.
--
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] HTTPCLIENT-2159: Mitigating Issues with Charset Handling During Redirects [httpcomponents-core]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #483:
URL: https://github.com/apache/httpcomponents-core/pull/483#issuecomment-2369323519
> > I think we should use a builder for ContenType instead of piling on constructors or factory methods.
>
> HI @garydgregory That seems like a massive change. Maybe we can consider a full refactor of the class in another ticket? I'm not sure if it's necessary right now compared to the charset ContentType parameter handling.
Hi @arturobernalg
Whatever the scale of the change it kind of feels like the right time to do it IMO _because_ we are taking of new factory methods/construction.
I do take your point that it could be done in a separate PR _if_ that PR happens first. If we agree on implementing a builder, I can volunteer to do it unless you'd rather do it.
--
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