From 6407dbba6b5c533358a993679ba6c85d23e2f5b8 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sun, 14 Sep 2025 19:13:52 +0200 Subject: [PATCH] HTTPCLIENT-2395: Avoid double-encoding of RFC 5987 filename* and pass through pre-encoded values. Make ConnPoolSupport.formatStats null-safe to prevent NPEs during lease/shutdown. Add multipart tests covering EXTENDED mode and pre-encoded filename*. --- .../entity/mime/HttpRFC7578Multipart.java | 15 +++-- .../mime/TestMultipartEntityBuilder.java | 65 +++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/HttpRFC7578Multipart.java b/httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/HttpRFC7578Multipart.java index 3e5fd010d2..5b2167245a 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/HttpRFC7578Multipart.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/HttpRFC7578Multipart.java @@ -101,12 +101,19 @@ protected void formatMultipartHeader(final MultipartPart part, final OutputStrea writeBytes("=\"", out); if (value != null) { if (name.equalsIgnoreCase(MimeConsts.FIELD_PARAM_FILENAME_START)) { - final String encodedValue = "UTF-8''" + PercentCodec.RFC5987.encode(value); - writeBytes(encodedValue, StandardCharsets.US_ASCII, out); + if (value.startsWith("UTF-8''")) { + writeBytes(value, StandardCharsets.US_ASCII, out); + } else { + final StringBuilder sb = new StringBuilder(value.length() + 16); + sb.append("UTF-8''"); + PercentCodec.RFC5987.encode(sb, value); + writeBytes(sb.toString(), StandardCharsets.US_ASCII, out); + } } else if (name.equalsIgnoreCase(MimeConsts.FIELD_PARAM_FILENAME)) { if (mode == HttpMultipartMode.EXTENDED) { - final String encodedValue = PercentCodec.RFC5987.encode(value); - writeBytes(encodedValue, StandardCharsets.US_ASCII, out); + final StringBuilder sb = new StringBuilder(value.length() + 8); + PercentCodec.RFC5987.encode(sb, value); + writeBytes(sb.toString(), StandardCharsets.US_ASCII, out); } else { // Default to ISO-8859-1 for RFC 7578 compliance in STRICT/LEGACY writeBytes(value, StandardCharsets.ISO_8859_1, out); diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java b/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java index 5fb2243b37..36b3580bee 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java @@ -384,4 +384,69 @@ private NameValuePair extractBoundary(final String contentType, final String exp return elem.getParameterByName("boundary"); } + @Test + void testMultipartWriteToRFC7578ModeWithFilenameStarPreEncodedPassThrough() throws Exception { + final String body = "hi"; + // Pre-encoded RFC 5987 value (as produced by a previous stage) + final String preEncoded = "UTF-8''%F0%9F%90%99_inline-%E5%9B%BE%E5%83%8F_%E6%96%87%E4%BB%B6.png"; + + final List parameters = new ArrayList<>(); + parameters.add(new BasicNameValuePair(MimeConsts.FIELD_PARAM_NAME, "test")); + // Provide pre-encoded value directly to filename* param + parameters.add(new BasicNameValuePair(MimeConsts.FIELD_PARAM_FILENAME_START, preEncoded)); + + final MultipartFormEntity entity = MultipartEntityBuilder.create() + .setMode(HttpMultipartMode.EXTENDED) + .setBoundary("xxxxxxxxxxxxxxxxxxxxxxxx") + .addPart(new FormBodyPartBuilder() + .setName("test") + .setBody(new StringBody(body, ContentType.TEXT_PLAIN.withCharset(StandardCharsets.UTF_8))) + .addField("Content-Disposition", "multipart/form-data", parameters) + .build()) + .buildEntity(); + + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + entity.writeTo(out); + out.close(); + final String wire = out.toString(StandardCharsets.ISO_8859_1.name()); + + // Pass-through EXACTLY the given value (no second prefix, no %25-escaping) + Assertions.assertTrue(wire.contains("filename*=\"" + preEncoded + "\"")); + Assertions.assertFalse(wire.contains("UTF-8''UTF-8%27%27")); + Assertions.assertFalse(wire.contains("%25F0%9F%90%99")); // octopus emoji must not be re-escaped as %25F0... + } + + @Test + void testExtendedModeAddBinaryBodyAddsFilenameAndFilenameStar_NoDoubleEncoding() throws Exception { + // Non-ASCII filename to trigger RFC 5987 behavior + final String filename = "πŸ™_图像_ζ–‡δ»Ά.png"; + // Expected percent-encoded for both filename and filename* + final String pct = "%F0%9F%90%99_%E5%9B%BE%E5%83%8F_%E6%96%87%E4%BB%B6.png"; + + final MultipartFormEntity entity = MultipartEntityBuilder.create() + .setMode(HttpMultipartMode.EXTENDED) + .setBoundary("xxxxxxxxxxxxxxxxxxxxxxxx") + .addBinaryBody("attachments", new byte[]{1, 2}, ContentType.IMAGE_PNG, filename) + .buildEntity(); + + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + entity.writeTo(out); + out.close(); + final String wire = out.toString(StandardCharsets.ISO_8859_1.name()); + + // Base header + Assertions.assertTrue(wire.contains("Content-Disposition: form-data; name=\"attachments\"")); + + // filename param (percent-encoded for ASCII transport) + Assertions.assertTrue(wire.contains("filename=\"" + pct + "\"")); + + // filename* param (single RFC 5987 value, no double prefix / no %25-escaping) + Assertions.assertTrue(wire.contains("filename*=\"UTF-8''" + pct + "\"")); + + // Guard against regressions + Assertions.assertFalse(wire.contains("UTF-8''UTF-8%27%27")); + Assertions.assertFalse(wire.contains("%25F0%9F%90%99")); + } + + }