feat(fuzz): enhance MultiPartForm with metadata APIs

[dwisiswant0]

• add SetFileMetadata/GetFileMetadata APIs for file metadata management.

• implement RFC-2046 boundary validation (max 70 chars).

• add boundary validation in Decode method.

• fix filesMetadata initialization.

• fix mem leak by removing defer from file reading loop.

• fix file metadata overwriting by storing first file's metadata instead of last.

Closes #6405, #6406

Proposed changes

Checklist

• Pull request is created against the dev branch
• All checks passed (lint, unit/integration/regression tests etc.) with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Summary by CodeRabbit

• New Features

  • Added per-field file metadata support with public getters/setters; preserves the first file’s metadata when multiple files share a field.

• Bug Fixes

  • Enforces RFC-compliant maximum boundary length (70 chars).
  • Decoding now requires a set boundary, returning a clear error if missing.
  • Ensures metadata storage is initialized to avoid nil-map issues.
  • Explicitly closes file resources on all success/error paths.

• Tests

  • Added tests covering metadata handling, boundary parsing/validation, missing-boundary errors, multi-file decoding, and nil-map behavior.

[coderabbitai]

Walkthrough

Adds per-field file metadata storage and accessor methods to MultiPartForm, enforces multipart boundary presence and 70-char maximum, updates Decode/Parse to collect and retain first-file metadata per field, initializes metadata map in constructor, and replaces deferred file closes with explicit Close calls; tests added.

Changes

Cohort / File(s)	Summary
Multipart form implementation pkg/fuzz/dataformat/multipart.go	Added filesMetadata map; constructor ensures map initialization; added SetFileMetadata and GetFileMetadata; ParseBoundary enforces boundary presence and 70-char max; Decode/Parse collect per-file metadata, store the first file's metadata per field, require boundary be set, and replace deferred Close() with explicit Close() calls.
Tests for metadata and boundary handling pkg/fuzz/dataformat/multipart_test.go	New tests covering map initialization, SetFileMetadata/GetFileMetadata behavior (including nil-map handling), boundary parsing success/failure (missing and >70 chars), Decode error when boundary unset, and Decode with multiple files verifying content and first-file metadata retention.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant MultiPartForm
  participant ParseBoundary
  participant Reader as multipart.Reader

  Caller->>MultiPartForm: NewMultiPartForm()
  note right of MultiPartForm #DDEBF7: filesMetadata map initialized

  Caller->>MultiPartForm: SetFileMetadata(field, metadata)
  MultiPartForm-->>Caller: ack

  Caller->>MultiPartForm: ParseBoundary(rawHeader)
  MultiPartForm->>ParseBoundary: validate presence & length (<=70)
  ParseBoundary-->>MultiPartForm: boundary | error

  Caller->>MultiPartForm: Decode(body)
  MultiPartForm->>Reader: NewReader(body, boundary)
  loop parts
    Reader-->>MultiPartForm: NextPart()
    MultiPartForm->>MultiPartForm: read bytes, collect per-file metadata
    MultiPartForm->>MultiPartForm: explicit Close() on part
  end
  MultiPartForm->>MultiPartForm: store first file metadata per field
  MultiPartForm-->>Caller: decoded form or error

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• Mzack9999

Poem

A boundary neat, not long nor wild,
I nibble parts like a careful child.
I keep the first file's quiet song,
Close each door—no defers prolong.
Metadata snug in a map I made,
Hop—tests pass green in the glade.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes Check	⚠️ Warning	While the PR delivers the requested APIs, it also introduces additional behavioral changes not specified in the linked issue, including RFC-2046 boundary length enforcement, requiring ParseBoundary before Decode, changed file-close handling, and selecting the first file's metadata instead of the last. Those modifications go beyond the scope of [#6405] and may affect backward compatibility or existing consumers. Therefore this check fails because the PR contains out-of-scope changes that need explicit justification or separation.	Ask the author to either split the non-API behavioral changes into a separate PR or explicitly document and justify them in this PR (including linked issue references), and add targeted tests demonstrating backward-compatibility or intended breaking behavior. Reviewers should verify the boundary validation, Decode precondition, and metadata-selection change are intentional and safe before merge. Update the PR description and changelog to call out these behavioral changes so reviewers can evaluate impact.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly and accurately summarizes the primary intent of the changes—adding metadata APIs to MultiPartForm—and follows conventional commit style (feat/fuzz). It is concise, single-sentence, and directly related to the main changes in the diff. Although the PR also includes boundary validation and other fixes, the title correctly highlights the main feature added.
Linked Issues Check	✅ Passed	The PR implements the API additions requested in [#6405] by adding SetFileMetadata and GetFileMetadata with the expected signatures and includes tests and an initialization fix (filesMetadata non-nil) to support those APIs. The provided summaries show the methods and test coverage were added and exercised, matching the coding objectives of the linked issue. Based on the changeset and tests, the linked-issue requirements are satisfied.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dwisiswant0/feat/fuzz/enhance-MultiPartForm-with-metadata-APIs

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (7)

pkg/fuzz/dataformat/multipart.go (5)

61-68: Encode: don’t require callers to pre‑set boundary.

If boundary is empty, let multipart.Writer pick one and persist it; only call SetBoundary when non‑empty.

Apply:

 w := multipart.NewWriter(&b)
-if err := w.SetBoundary(m.boundary); err != nil {
-  return "", err
-}
+if m.boundary != "" {
+  if err := w.SetBoundary(m.boundary); err != nil {
+    return "", err
+  }
+} else {
+  // persist auto-generated boundary for Decode/Content-Type construction
+  m.boundary = w.Boundary()
+}

---

74-96: Encode only treats []any as “files”; support []string/single string too.

Today file parts are created only when value is []any; []string or string falls back to form fields, which is surprising if metadata exists.

Apply:

-    if fileMetadata, ok := m.filesMetadata[key]; ok {
-      if filesArray, isArray := value.([]any); isArray {
-        for _, file := range filesArray {
+    if fileMetadata, ok := m.filesMetadata[key]; ok {
+      // []string
+      if filesStr, ok := value.([]string); ok {
+        for _, content := range filesStr {
           h := make(textproto.MIMEHeader)
           h.Set("Content-Disposition",
             fmt.Sprintf(`form-data; name=%q; filename=%q`,
               key, fileMetadata.Filename))
           h.Set("Content-Type", fileMetadata.ContentType)
-          if fw, err = w.CreatePart(h); err != nil {
+          if fw, err = w.CreatePart(h); err != nil {
             Itererr = err
             return false
           }
-          if _, err = fw.Write([]byte(file.(string))); err != nil {
+          if _, err = fw.Write([]byte(content)); err != nil {
             Itererr = err
             return false
           }
         }
         return true
+      }
+      // []any (fallback)
+      if filesAny, ok := value.([]any); ok {
+        for _, v := range filesAny {
+          content, ok := v.(string)
+          if !ok {
+            Itererr = fmt.Errorf("file content for %q must be string, got %T", key, v)
+            return false
+          }
+          h := make(textproto.MIMEHeader)
+          h.Set("Content-Disposition",
+            fmt.Sprintf(`form-data; name=%q; filename=%q`, key, fileMetadata.Filename))
+          h.Set("Content-Type", fileMetadata.ContentType)
+          if fw, err = w.CreatePart(h); err != nil {
+            Itererr = err
+            return false
+          }
+          if _, err = fw.Write([]byte(content)); err != nil {
+            Itererr = err
+            return false
+          }
+        }
+        return true
+      }
+      // single string
+      if content, ok := value.(string); ok {
+        h := make(textproto.MIMEHeader)
+        h.Set("Content-Disposition",
+          fmt.Sprintf(`form-data; name=%q; filename=%q`, key, fileMetadata.Filename))
+        h.Set("Content-Type", fileMetadata.ContentType)
+        if fw, err = w.CreatePart(h); err != nil {
+          Itererr = err
+          return false
+        }
+        if _, err = fw.Write([]byte(content)); err != nil {
+          Itererr = err
+          return false
+        }
+        return true
       }
     }

---

151-156: Boundary length check — LGTM; consider validating charset too.

Optional: reuse stdlib’s validation to catch illegal chars.

Apply:

   m.boundary = params["boundary"]
   if m.boundary == "" {
     return fmt.Errorf("no boundary found in the content type")
   }
   // RFC‑2046: ≤ 70 chars
   if len(m.boundary) > 70 {
     return fmt.Errorf("boundary exceeds maximum length of 70 characters")
   }
+  // piggyback stdlib validation of allowed characters
+  w := multipart.NewWriter(io.Discard)
+  defer w.Close()
+  if err := w.SetBoundary(m.boundary); err != nil {
+    return fmt.Errorf("invalid boundary: %w", err)
+  }

---

201-208: Check file.Close() errors and avoid double‑return paths.

Small robustness win; Close can fail on some backends.

Apply:

- buffer := new(bytes.Buffer)
- if _, err := buffer.ReadFrom(file); err != nil {
-   file.Close()
-   return KV{}, err
- }
- file.Close()
+ buffer := new(bytes.Buffer)
+ if _, err := io.Copy(buffer, file); err != nil {
+   _ = file.Close()
+   return KV{}, err
+ }
+ if err := file.Close(); err != nil {
+   return KV{}, err
+ }

---

219-223: Store “first” file metadata — LGTM; document overwrite semantics.

Decode overwrites any pre‑set metadata for the same key; call out this inbound vs. outbound behavior in the type’s docs to avoid surprises.

pkg/fuzz/dataformat/multipart_test.go (2)

278-300: Simplify “long boundary” generation.

Current build mutates strings in a loop with zero bytes; simpler and clearer with strings.Repeat.

Apply:

-// Test boundary too long (over 70 characters)
-longBoundary := "multipart/form-data; boundary=" + string(make([]byte, 71))
-for i := range longBoundary[len("multipart/form-data; boundary="):] {
-  longBoundary = longBoundary[:len("multipart/form-data; boundary=")+i] + "a" + longBoundary[len("multipart/form-data; boundary=")+i+1:]
-}
+// Test boundary too long (over 70 characters)
+longBoundary := "multipart/form-data; boundary=" + strings.Repeat("a", 71)

Remember to import:

-import (
+import (
   "testing"
+  "strings"

---

11-139: Missing encode‑with‑metadata test.

Add a round‑trip test that sets metadata, encodes files, then decodes and asserts both content and that GetFileMetadata returns the first file’s metadata. This exercises CreatePart path.

I can draft this test if you want.

Also applies to: 156-166

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f610ed4 and 27b8172.

📒 Files selected for processing (2)

• pkg/fuzz/dataformat/multipart.go (3 hunks)
• pkg/fuzz/dataformat/multipart_test.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt
Run static analysis with go vet

Files:

• pkg/fuzz/dataformat/multipart_test.go
• pkg/fuzz/dataformat/multipart.go

🧬 Code graph analysis (2)

pkg/fuzz/dataformat/multipart_test.go (1)

pkg/fuzz/dataformat/multipart.go (3)

• NewMultiPartForm (29-33)
• FileMetadata (19-22)
• MultiPartForm (14-17)

pkg/fuzz/dataformat/multipart.go (1)

pkg/fuzz/dataformat/kv.go (1)

• KV (15-18)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Lint

🔇 Additional comments (11)

pkg/fuzz/dataformat/multipart.go (5)

30-33: Constructor now nil-safe for metadata — LGTM.

Initializing filesMetadata in NewMultiPartForm avoids nil map hazards later.

---

35-42: SetFileMetadata nil-guard — LGTM.

Correctly initializes on first use if someone bypasses the constructor.

---

44-53: GetFileMetadata zero‑value behavior — LGTM.

Returns (zero, false) when map is nil or key missing; idiomatic.

---

162-165: Decode requires boundary — LGTM.

Fail‑fast guard prevents subtle parse errors.

---

187-190: Nil‑safe metadata map before population — LGTM.

pkg/fuzz/dataformat/multipart_test.go (6)

243-261: Metadata API tests — LGTM.

Covers both hit/miss paths for Set/Get.

---

263-276: Constructor initialization test — LGTM.

Verifies non‑nil map and round‑trip of stored entry.

---

302-309: Decode requires boundary — LGTM.

Asserts the new guard properly.

---

311-347: Multiple files keep first metadata — LGTM.

Validates first one wins and content aggregation.

---

348-362: SetFileMetadata with nil receiver map — LGTM.

---

364-371: GetFileMetadata with nil map — LGTM.

[dogancanbakir]

lint errors need to be fixed

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (7)

pkg/fuzz/dataformat/multipart.go (7)

35-43: SetFileMetadata nil‑map guard is good; verify concurrency expectations

If MultiPartForm may be used across goroutines, guard map access to avoid races; otherwise document it as not thread‑safe.

Optional patch:

--- a/pkg/fuzz/dataformat/multipart.go
+++ b/pkg/fuzz/dataformat/multipart.go
@@
 import (
 	"bytes"
 	"fmt"
 	"io"
 	"mime"
 	"mime/multipart"
 	"net/textproto"
+	"sync"
@@
 type MultiPartForm struct {
 	boundary      string
 	filesMetadata map[string]FileMetadata
+	mu            sync.RWMutex
 }
@@
 func (m *MultiPartForm) SetFileMetadata(fieldName string, metadata FileMetadata) {
-	if m.filesMetadata == nil {
-		m.filesMetadata = make(map[string]FileMetadata)
-	}
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.filesMetadata == nil {
+		m.filesMetadata = make(map[string]FileMetadata)
+	}
 	m.filesMetadata[fieldName] = metadata
 }
@@
 func (m *MultiPartForm) GetFileMetadata(fieldName string) (FileMetadata, bool) {
-	if m.filesMetadata == nil {
-		return FileMetadata{}, false
-	}
-	metadata, exists := m.filesMetadata[fieldName]
-	return metadata, exists
+	m.mu.RLock()
+	defer m.mu.RUnlock()
+	if m.filesMetadata == nil {
+		return FileMetadata{}, false
+	}
+	metadata, exists := m.filesMetadata[fieldName]
+	return metadata, exists
 }

---

151-156: RFC‑2046 boundary length check — LGTM; consider token validation

Optional: reject non‑token characters to harden parsing (boundary must be a “token” per RFC 2045/2046).

---

201-208: Close error is silently discarded; return it when read succeeds

Capture file.Close() error so I/O errors aren’t lost.

Apply:

-			buffer := new(bytes.Buffer)
-			if _, err := buffer.ReadFrom(file); err != nil {
-				_ = file.Close()
-				return KV{}, err
-			}
-			_ = file.Close()
+			buffer := new(bytes.Buffer)
+			_, readErr := buffer.ReadFrom(file)
+			closeErr := file.Close()
+			if readErr != nil {
+				return KV{}, readErr
+			}
+			if closeErr != nil {
+				return KV{}, closeErr
+			}

---

219-224: Store first file’s metadata — LGTM; optional: keep all

Meets the PR goal; if future callers need per‑file metadata, switch to map[string][]FileMetadata.

---

74-96: Encoding with metadata only supports []any; extend to []string/string

With metadata set, passing []string or string currently creates form fields instead of files. Support common types to reduce foot‑guns.

Apply:

-		if fileMetadata, ok := m.filesMetadata[key]; ok {
-			if filesArray, isArray := value.([]any); isArray {
+		if fileMetadata, ok := m.filesMetadata[key]; ok {
+			if filesArray, isArray := value.([]any); isArray {
 				for _, file := range filesArray {
 					h := make(textproto.MIMEHeader)
 					h.Set("Content-Disposition",
 						fmt.Sprintf(`form-data; name=%q; filename=%q`,
 							key, fileMetadata.Filename))
 					h.Set("Content-Type", fileMetadata.ContentType)
 
 					if fw, err = w.CreatePart(h); err != nil {
 						Itererr = err
 						return false
 					}
 
 					if _, err = fw.Write([]byte(file.(string))); err != nil {
 						Itererr = err
 						return false
 					}
 				}
 
 				return true
-			}
+			} else if strSlice, ok := value.([]string); ok {
+				for _, file := range strSlice {
+					h := make(textproto.MIMEHeader)
+					h.Set("Content-Disposition",
+						fmt.Sprintf(`form-data; name=%q; filename=%q`,
+							key, fileMetadata.Filename))
+					h.Set("Content-Type", fileMetadata.ContentType)
+					if fw, err = w.CreatePart(h); err != nil {
+						Itererr = err
+						return false
+					}
+					if _, err = fw.Write([]byte(file)); err != nil {
+						Itererr = err
+						return false
+					}
+				}
+				return true
+			} else if s, ok := value.(string); ok {
+				h := make(textproto.MIMEHeader)
+				h.Set("Content-Disposition",
+					fmt.Sprintf(`form-data; name=%q; filename=%q`,
+						key, fileMetadata.Filename))
+				h.Set("Content-Type", fileMetadata.ContentType)
+				if fw, err = w.CreatePart(h); err != nil {
+					Itererr = err
+					return false
+				}
+				if _, err = fw.Write([]byte(s)); err != nil {
+					Itererr = err
+					return false
+				}
+				return true
+			}
 		}

---

136-138: Check error from Writer.Close()

Surface encoding errors instead of dropping them.

Apply:

-	_ = w.Close()
-	return b.String(), nil
+	if err := w.Close(); err != nil {
+		return "", err
+	}
+	return b.String(), nil

---

55-59: IsType stub

If MultiPartForm should participate in auto‑detection, implement a cheap detector (e.g., prefix “--” boundary lines). Otherwise, consider a TODO explaining why it’s intentionally false.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27b8172 and d5765a5.

📒 Files selected for processing (1)

• pkg/fuzz/dataformat/multipart.go (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt
Run static analysis with go vet

Files:

• pkg/fuzz/dataformat/multipart.go

🧬 Code graph analysis (1)

pkg/fuzz/dataformat/multipart.go (2)

pkg/fuzz/dataformat/dataformat.go (1)

• Decode (72-87)

pkg/fuzz/dataformat/kv.go (1)

• KV (15-18)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Lint

🔇 Additional comments (5)

pkg/fuzz/dataformat/multipart.go (5)

30-33: Constructor initializes filesMetadata — LGTM

Prevents nil-map panics and simplifies callers.

---

44-53: GetFileMetadata semantics — LGTM

Zero‑value + bool is clear and idiomatic.

---

162-164: Decode precondition for boundary — LGTM

Clear error guides callers to ParseBoundary first.

---

186-190: Decode: filesMetadata lazy init — LGTM

Avoids nil map writes when NewMultiPartForm wasn’t used.

---

1-232: fmt/vet/tests quick pass — blocked by build error

go vet / go test failed with: pkg/output/stats/waf/waf.go:27:12 — pattern "regexes.json" not found. Add the missing embedded file or adjust build tags so the package builds, then re-run: gofmt -l pkg/fuzz/dataformat/multipart.go; go vet ./...; go test ./pkg/fuzz/dataformat -run 'MultiPart|Multipart|FileMetadata|Boundary' -v; rg -n --type go 'SetFileMetadata|GetFileMetadata' -C2