Repository navigation
Conversation
Madrigal
marked this pull request as ready for review
October 22, 2026 17:58
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #, if available: #3891
Description of changes:
XML request payloads (S3, S3 CRT, S3 Control, CloudFront, Route 53) were serialized with
XmlDocument::ConvertToString(), which uses tinyxml2's default pretty-printing. That adds indentation and newlines to every element. For aCompleteMultipartUploadwith 9,331 parts the body is 1,090,743 bytes (1.04 MiB). Some S3-compatible services with a 1 MiB request/XML limit, such as Ceph RGW, reject it even though the upload is under the 10,000-part maximum.Changes:
XmlDocument::ConvertToCompactString(), which prints the same document without whitespace between elements. Whitespace inside text content is unchanged.ConvertToString()is unchanged and still pretty-prints, since it is public API and may be used outside the SDK, e.g. to save a document to a file. Both methods' doc comments now point to each other.XmlRequestSource.vm, S3PutBucketNotificationConfigurationRequest.vm) toConvertToCompactString(), and regenerate the affected clients. That's 190 files, each a one-line change:payloadDoc.ConvertToString()→payloadDoc.ConvertToCompactString().Result: the 9,331-part
CompleteMultipartUploadbody shrinks to 829,472 bytes (0.79 MiB), and 10,000 parts comes to 0.85 MiB.This covers
S3ClientandS3CrtClient::CompleteMultipartUpload, and Transfer Manager v1. Automatic multipart uploads throughS3CrtClient::PutObjectand the S3 Transfer Manager v2 build this body inside aws-c-s3, which hard-codes the same whitespace. That needs a separate fix in awslabs/aws-c-s3.This also replaces the approach in #3892 (a
ClientConfigurationflag), which was closed in review in favor of not threading serializer options through requests.Check all that applies:
semantically identical XML.
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.