Repository navigation
fix(multipart): omit nil header string values and support additionalValuesToMerge - #2220
Conversation
|
I've opened this PR in
Unit tests covering header omission, additional values merging, and invalid part validation have been added, and |
| if (onStartObjectSerialization != null) { | ||
| onStartObjectSerialization.accept(additionalValueToMerge, this); | ||
| } | ||
| additionalValueToMerge.serialize(this); |
There was a problem hiding this comment.
Updated. Serialized parts now share a single boundary (using the root body's boundary) with intermediate closing delimiters suppressed, emitting only a single final boundary delimiter. Replaced substring checks with boundary-aware assertions verifying the exact multi-part structure and ensuring no secondary boundaries leak.
| final byte[] bytes = result.readAllBytes(); | ||
| assertEquals("\r\n", new String(bytes, "UTF-8")); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void writeStringValueWithEmptyKeyAndValueWritesRawLine() throws IOException { | ||
| try (final var serializationWriter = new MultipartSerializationWriter()) { | ||
| serializationWriter.writeStringValue("", "raw line"); | ||
| try (final var result = serializationWriter.getSerializedContent()) { | ||
| final byte[] bytes = result.readAllBytes(); |
There was a problem hiding this comment.
Updated. Replaced InputStream.readAllBytes() with Compatibility.readAllBytes() to preserve compatibility with Android API < 33.
| final MultipartBody part1 = new MultipartBody(); | ||
| final TestEntity nonMultipart = new TestEntity(); | ||
|
|
||
| try (final var serializationWriter = new MultipartSerializationWriter()) { | ||
| assertThrows( | ||
| RuntimeException.class, | ||
| () -> serializationWriter.writeObjectValue(null, part1, nonMultipart)); |
There was a problem hiding this comment.
Updated. Passed null for the primary body in throwsOnInvalidAdditionalValuesToMerge to isolate validation of additionalValuesToMerge, and added an assertion on the exact exception message.
…ress review feedback
|
I have updated the PR in commit \29b7efa0\ to address the review recommendations:
Local Gradle test suites and \spotlessCheck\ pass cleanly. Whenever convenient, please take another look. Thank you! |
Summary of Changes
Omit Nil Header String Values in
writeStringValue:writeStringValue(key, value), ifkey != null && !key.isEmpty()andvalue == null, it previously serializedkey + "\r\n"without a colon or value, creating a malformed MIME header line that violates RFC 2046 and RFC 7230/5322 syntax.if (key != null && !key.isEmpty() && value == null), returning immediately so that headers with null values are cleanly omitted, matching the Kiota serialization convention across SDKs.key == null || key.isEmpty()andvalue == null("\r\n").Support
additionalValuesToMergeinwriteObjectValue:writeObjectValuevalidatedObjects.requireNonNull(additionalValuesToMerge)but completely ignored the array without serializing any additionalMultipartBodyparts.MultipartBody), invoking lifecycle callbacks (onBeforeObjectSerialization,onStartObjectSerialization,onAfterObjectSerialization), and serializing each part (consistent withFormSerializationWriterandJsonSerializationWriter).Unit Tests Added in
MultipartSerializationWriterTests:writesStringValueWithKeyValue: verifies standard header serialization (Content-Type: text/plain).writeStringValueWithNilValueOmitsHeader: verifies headers with null values are omitted.writeStringValueWithEmptyKeyAndNilValueWritesDelimiter: verifies delimiter emission (\r\n).writeStringValueWithEmptyKeyAndValueWritesRawLine: verifies raw line serialization.writesAStructuredObjectWithAdditionalValuesToMerge: verifies merging multipleMultipartBodyparts.throwsOnInvalidAdditionalValuesToMerge: asserts runtime exception for non-MultipartBody instances in additional values.