Unverified Commit 4cf4265a authored by Kefu Chai's avatar Kefu Chai Committed by GitHub
Browse files

fix(s3s/ops): Return MalformedXML for empty XML body in operations requiring it (#377) 



According to AWS S3 API specifications, certain operations require an XML
request body and should return MalformedXML (HTTP 400) when the body is
empty or missing. This commit implements validation for three such operations:

1. CompleteMultipartUpload: Requires XML body with list of uploaded parts
2. PutObjectLegalHold: Requires XML body with LegalHold status (ON/OFF)
3. PutObjectRetention: Requires XML body with retention Mode and RetainUntilDate

The fix is implemented in the code generator (s3s_codegen) to ensure
consistency across both generated.rs and generated_minio.rs files.

Operations with truly optional XML bodies (CreateBucket, PutBucketAcl,
PutObjectAcl, PutBucketLifecycleConfiguration, RestoreObject) are left
unchanged as they can legitimately have empty bodies when using alternative
request methods (e.g., canned ACLs via headers, default region creation).

Signed-off-by: default avatarKefu Chai <tchaikov@gmail.com>
parent 5b07a6d7

codegen/src/v1/ops.rs

+23 −1
Original line number Diff line number Diff line
@@ -498,7 +498,29 @@ fn codegen_op_http_de(op: &Operation, rust_types: &RustTypes) { 
                                g!("let {}: Option<{}> = Some(http::take_stream_body(req));", field.name, field.type_);

                            }

                            _ => {

                                if field.option_type {

                                // AWS S3 returns MalformedXML for empty bodies on these operations,

                                // which differs from the default behavior where empty optional XML bodies are accepted.

                                // - CompleteMultipartUpload: requires XML body with list of uploaded parts

                                // - PutObjectLegalHold: requires XML body with ON/OFF legal hold status

                                // - PutObjectRetention: requires XML body with retention mode and date

                                let requires_body = matches!(

                                    (op.name.as_str(), field.name.as_str()),

                                    ("CompleteMultipartUpload", "multipart_upload")

                                        | ("PutObjectLegalHold", "legal_hold")

                                        | ("PutObjectRetention", "retention")

                                );



                                if requires_body {

                                    // These operations require XML body to match AWS S3 behavior; empty body should return MalformedXML instead of being treated as optional

                                    assert!(field.option_type);

                                    g!("let {}: Option<{}> = match http::take_xml_body(req) {{", field.name, field.type_);

                                    g!("    Ok(body) => Some(body),");

                                    g!("    Err(e) if *e.code() == crate::S3ErrorCode::MissingRequestBodyError => {{");

                                    g!("        return Err(crate::S3ErrorCode::MalformedXML.into());");

                                    g!("    }}");

                                    g!("    Err(e) => return Err(e),");

                                    g!("}};");

                                } else if field.option_type {

                                    g!("let {}: Option<{}> = http::take_opt_xml_body(req)?;", field.name, field.type_);

                                } else {

                                    g!("let {}: {} = http::take_xml_body(req)?;", field.name, field.type_);

crates/s3s/src/ops/generated.rs

+21 −3
Original line number Diff line number Diff line
@@ -555,7 +555,13 @@ impl CompleteMultipartUpload { 


        let mpu_object_size: Option<MpuObjectSize> = http::parse_opt_header(req, &X_AMZ_MP_OBJECT_SIZE)?;



        let multipart_upload: Option<CompletedMultipartUpload> = http::take_opt_xml_body(req)?;

        let multipart_upload: Option<CompletedMultipartUpload> = match http::take_xml_body(req) {

            Ok(body) => Some(body),

            Err(e) if *e.code() == crate::S3ErrorCode::MissingRequestBodyError => {

                return Err(crate::S3ErrorCode::MalformedXML.into());

            }

            Err(e) => return Err(e),

        };



        let request_payer: Option<RequestPayer> = http::parse_opt_header(req, &X_AMZ_REQUEST_PAYER)?;
@@ -5722,7 +5728,13 @@ impl PutObjectLegalHold { 


        let expected_bucket_owner: Option<AccountId> = http::parse_opt_header(req, &X_AMZ_EXPECTED_BUCKET_OWNER)?;



        let legal_hold: Option<ObjectLockLegalHold> = http::take_opt_xml_body(req)?;

        let legal_hold: Option<ObjectLockLegalHold> = match http::take_xml_body(req) {

            Ok(body) => Some(body),

            Err(e) if *e.code() == crate::S3ErrorCode::MissingRequestBodyError => {

                return Err(crate::S3ErrorCode::MalformedXML.into());

            }

            Err(e) => return Err(e),

        };



        let request_payer: Option<RequestPayer> = http::parse_opt_header(req, &X_AMZ_REQUEST_PAYER)?;
@@ -5850,7 +5862,13 @@ impl PutObjectRetention { 


        let request_payer: Option<RequestPayer> = http::parse_opt_header(req, &X_AMZ_REQUEST_PAYER)?;



        let retention: Option<ObjectLockRetention> = http::take_opt_xml_body(req)?;

        let retention: Option<ObjectLockRetention> = match http::take_xml_body(req) {

            Ok(body) => Some(body),

            Err(e) if *e.code() == crate::S3ErrorCode::MissingRequestBodyError => {

                return Err(crate::S3ErrorCode::MalformedXML.into());

            }

            Err(e) => return Err(e),

        };



        let version_id: Option<ObjectVersionId> = http::parse_opt_query(req, "versionId")?;

crates/s3s/src/ops/generated_minio.rs

+21 −3
Original line number Diff line number Diff line
@@ -555,7 +555,13 @@ impl CompleteMultipartUpload { 


        let mpu_object_size: Option<MpuObjectSize> = http::parse_opt_header(req, &X_AMZ_MP_OBJECT_SIZE)?;



        let multipart_upload: Option<CompletedMultipartUpload> = http::take_opt_xml_body(req)?;

        let multipart_upload: Option<CompletedMultipartUpload> = match http::take_xml_body(req) {

            Ok(body) => Some(body),

            Err(e) if *e.code() == crate::S3ErrorCode::MissingRequestBodyError => {

                return Err(crate::S3ErrorCode::MalformedXML.into());

            }

            Err(e) => return Err(e),

        };



        let request_payer: Option<RequestPayer> = http::parse_opt_header(req, &X_AMZ_REQUEST_PAYER)?;
@@ -5737,7 +5743,13 @@ impl PutObjectLegalHold { 


        let expected_bucket_owner: Option<AccountId> = http::parse_opt_header(req, &X_AMZ_EXPECTED_BUCKET_OWNER)?;



        let legal_hold: Option<ObjectLockLegalHold> = http::take_opt_xml_body(req)?;

        let legal_hold: Option<ObjectLockLegalHold> = match http::take_xml_body(req) {

            Ok(body) => Some(body),

            Err(e) if *e.code() == crate::S3ErrorCode::MissingRequestBodyError => {

                return Err(crate::S3ErrorCode::MalformedXML.into());

            }

            Err(e) => return Err(e),

        };



        let request_payer: Option<RequestPayer> = http::parse_opt_header(req, &X_AMZ_REQUEST_PAYER)?;
@@ -5865,7 +5877,13 @@ impl PutObjectRetention { 


        let request_payer: Option<RequestPayer> = http::parse_opt_header(req, &X_AMZ_REQUEST_PAYER)?;



        let retention: Option<ObjectLockRetention> = http::take_opt_xml_body(req)?;

        let retention: Option<ObjectLockRetention> = match http::take_xml_body(req) {

            Ok(body) => Some(body),

            Err(e) if *e.code() == crate::S3ErrorCode::MissingRequestBodyError => {

                return Err(crate::S3ErrorCode::MalformedXML.into());

            }

            Err(e) => return Err(e),

        };



        let version_id: Option<ObjectVersionId> = http::parse_opt_query(req, "versionId")?;