fix: upload attachments nested in rich messages and InputMedia thumbnails (#298)

[ingvarch]

Fixes #298, and a second attachment bug in the same code path that surfaced while fixing it.

1. Attachments nested in a rich message are never uploaded (Rich message attachments are never uploaded (attach:// inside InputRichMessage is dropped) #298)

buildRequestForm had no case for models.InputRichMessage, so the field fell through to addFormFieldDefault and was written with a plain json.Marshal. The attach:// references were serialized into the JSON, but no file part was ever created, so Telegram had nothing to resolve them against and the readers were never read.

The attach:// walking that addFormFieldInputMediaItem and addFormFieldInputMediaSlice do for top level InputMedia fields simply never reached anything nested inside a rich message.

Fixed by adding addFormFieldInputRichMessage, which uploads the referenced attachments before encoding the field:

• InputRichMessage.Media[].Media
• the media blocks: InputRichBlockAnimation, InputRichBlockAudio, InputRichBlockDocument, InputRichBlockPhoto, InputRichBlockVideo, InputRichBlockVoiceNote
• recursively through the container blocks that carry nested blocks: InputRichBlockList (per item), InputRichBlockBlockQuotation, InputRichBlockCollage, InputRichBlockSlideshow, InputRichBlockDetails

Wired into the type switch for both shapes the params use: models.InputRichMessage (sendRichMessage, sendRichMessageDraft) and *models.InputRichMessage (editMessageText, editEphemeralMessageText).

The upload half of addFormFieldInputMediaItem is extracted into addInputMediaAttachment so the walker reuses it instead of duplicating the attach:// handling. The encoding of the field itself is unchanged.

2. The thumbnail of an InputMedia is never uploaded

Same class of bug, found by tracing the rest of the attach:// handling.

InputFileUpload.MarshalJSON emitted "@", which is not a Bot API reference. At the top level of a request this never showed: a Thumbnail field there becomes its own form part named thumbnail and the marshalled value is not used. Nested inside an InputMedia, the marshalled value is what Telegram reads, so the thumbnail was broken twice over: an unusable reference, and no file part to point at.

Affects InputMediaVideo, InputMediaAnimation, InputMediaAudio, InputMediaDocument and InputPaidMediaVideo, and therefore sendMediaGroup, editMessageMedia, editEphemeralMessageMedia, sendPaidMedia and the media blocks of a rich message.

Fixed by:

• InputFileUpload.MarshalJSON now emits attach://, the reference Telegram actually resolves, with Filename doubling as the part name
• a GetThumbnail() accessor on the five types that carry one, alongside the existing GetMedia() and Attachment()
• addInputFileAttachment, called from addInputMediaAttachment, which uploads a nested *InputFileUpload and leaves an InputFileString (a file_id or an URL) to the encoder

A nil Data on a nested upload returns an error rather than panicking, matching the treatment of nil MediaAttachment from #296.

Compatibility

No API is removed or renamed. Two behaviour changes worth naming:

• InputFileUpload.MarshalJSON returns a different string. Nothing consumed the old "@" form, since Telegram rejects it; the only other place it appears is the payload printed by the debug handler.
• The five GetThumbnail() methods are new exported methods on existing types.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 97.76536% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.75%. Comparing base (5842ef8) to head (b85209e).

Files with missing lines	Patch %	Lines
build_request_form.go	97.35%	2 Missing and 2 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #299      +/-   ##
==========================================
+ Coverage   48.50%   51.75%   +3.25%     
==========================================
  Files          34       34              
  Lines        2835     2935     +100     
==========================================
+ Hits         1375     1519     +144     
+ Misses       1401     1370      -31     
+ Partials       59       46      -13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ingvarch]

@negasus Pls review 🙏

[negasus]

Reviewed this against the branch head (b19d161).

The direction is right and I verified the core fix end-to-end: with sendRichMessage and an attach:// reference inside InputRichMessage.Media, main writes no file part (that is #298) and this branch writes it. I also checked the block walker covers every InputRichBlock variant that can nest []InputRichBlock — List items, BlockQuotation, Collage, Slideshow, Details — and that ExpandableBlockQuotation, PullQuotation, Table, Thinking carry only RichText, so nothing is missed. The MarshalJSON change is safe for the top level: every models.InputFile param field is dispatched by dynamic type to addFormFieldInputFileUpload, which names the part after the field, so the marshalled value is never used there.

Two blockers before this can merge, though. Both are the typed-nil interface trap, and both are new on this branch — the equivalent code passes on main.

1. addInputFileAttachment panics on a typed-nil thumbnail

build_request_form.go:209. The type assertion only guards on !ok, but a *models.InputFileUpload nil pointer stored in the InputFile interface field passes it:

upload, ok := value.(*models.InputFileUpload)
if !ok {
	return nil
}
if readerIsNil(upload.Data) {  // nil dereference

var thumb *models.InputFileUpload      // filled in conditionally, left nil here
params := &SendMediaGroupParams{ChatID: 1, Media: []models.InputMedia{
	&models.InputMediaVideo{Media: "attach://v.mp4", MediaAttachment: r, Thumbnail: thumb},
}}
// panic: runtime error: invalid memory address or nil pointer dereference

On main the same input marshals to "thumbnail": null, because encoding/json writes null for a nil pointer instead of calling its MarshalJSON.

2. The media.Media == nil guard does not catch a typed-nil InputMedia

build_request_form.go:272. Same trap one level up: the interface is non-nil, so the guard passes and addInputMediaAttachment calls value.GetMedia() on a nil receiver.

var doc *models.InputMediaDocument
rm := models.InputRichMessage{Media: []models.InputRichMessageMedia{{ID: "1", Media: doc}}}
buildRequestForm(form, &SendRichMessageParams{ChatID: 1, RichMessage: rm})
// panic: runtime error: invalid memory address or nil pointer dereference

Both trigger on ordinary Go — a helper that returns a nil *InputFileUpload when there is no thumbnail, or a conditionally assigned media pointer. And since buildRequestForm runs in the un-recovered goroutine in rawRequest (raw_request.go:32), a panic there takes the whole process down. That is the same failure class #296 fixed, and the one the PR description says it matched ("returns an error rather than panicking"). A nil-pointer check at both sites is enough.

3. Thumbnail form-part names collide silently

build_request_form.go:212 — form.CreateFormFile(upload.Filename, upload.Filename) makes Filename the attach:// key. Two videos in one sendMediaGroup, each with Thumbnail: &models.InputFileUpload{Filename: "thumb.jpg", ...} (a very natural name), produce two parts with the same name and two JSON entries pointing at attach://thumb.jpg; Telegram resolves both to one file, so the second video silently gets the first one's thumbnail. If the same *InputFileUpload value is reused across items, the second io.Copy reads an already-drained reader and writes an empty part.

Unlike the Media reference, which the caller writes explicitly and can keep unique, this key is derived implicitly and the collision is invisible. Erroring on a duplicate part name, or deriving a unique key, would be worth it.

Nit

The new README paragraph says "A thumbnail is always a new upload, never a file_id or an URL", but addInputFileAttachment deliberately passes *InputFileString through, and Test_buildRequestForm_inputMediaThumbnail asserts that a https://…/thumb.jpg thumbnail is emitted as-is. The prose and the tested behaviour disagree.

[ingvarch]

@negasus Fixed

[aliskhannn]

When you'll approve this PR? We need these bug fixes

[ingvarch]

@negasus could take a look? We really need this.

[negasus]

Thanks for the fix — the core of it is right. The attach:// walker covers every InputRichBlock* variant that can actually carry media (Table, Map, Buttons, PullQuotation, ExpandableBlockQuotation genuinely have no nested media or blocks), all five InputFile thumbnail fields get a GetThumbnail(), and go vet / go test ./... pass.

A few things I'd like addressed before merging. The first three I'd consider blocking: two of them are the same panic this PR claims to have fixed, on paths it doesn't cover, and the third is a regression for existing callers. Details inline.

[negasus]

value.Data is dereferenced without a nil guard, so a typed-nil *models.InputFileUpload in a top-level models.InputFile field panics:

var thumb *models.InputFileUpload
b.SendVideo(ctx, &bot.SendVideoParams{
    ChatID:    1,
    Video:     &models.InputFileString{Data: "file_id"},
    Thumbnail: thumb,
})
// runtime error: invalid memory address or nil pointer dereference

The form is built in the goroutine in raw_request.go:31 with no recover, so this takes the process down. The PR introduces isNilValue and guards exactly this shape for nested thumbnails, and the CHANGELOG entry says a typed nil InputFile thumbnail "no longer panics" — which reads as covering this path too, but doesn't. One line at the top fixes it:

if isNilValue(value) {
    return fmt.Errorf("nil value for field %s", fieldName)
}

[ingvarch]

Fixed. isNilValue guard at the top of addFormFieldInputFileUpload, so it returns nil value for field thumbnail instead of panicking.

[negasus]

Same class of bug, also unguarded: a typed-nil element inside []models.InputMedia / []models.InputPaidMedia reaches here and then addInputMediaAttachment → value.GetMedia() on a nil receiver.

b.SendMediaGroup(ctx, &bot.SendMediaGroupParams{
    ChatID: 1,
    Media: []models.InputMedia{
        &models.InputMediaPhoto{Media: "file_id"},
        (*models.InputMediaPhoto)(nil),
    },
})
// panics

This PR adds precisely that guard (isNilValue(media.Media)) for InputRichMessage.Media below, so it'd be good to close the much more common media-slice path in the same pass.

[ingvarch]

Fixed. Guarded in addFormFieldInputMediaSlice (nil value for field media at index 1) and in addFormFieldInputMedia for the single-field case.

[negasus]

Making a duplicate part name a hard error turns a previously working pattern into a failed call: referencing one file from two entries under a single attach:// name.

// both entries point at the same part
Media: []models.InputMedia{
    &models.InputMediaPhoto{Media: "attach://p.jpg", MediaAttachment: r1},
    &models.InputMediaPhoto{Media: "attach://p.jpg"},
}
// now: duplicate form part name "p.jpg"

Before, two parts were written and Telegram resolved both references to the first — which is the intended result when the content is the same. Same story for two rich-message photo blocks sharing a name, and for []models.InputSticker (createNewStickerSet reusing one image across two emoji entries).

Reusing the already-written part when the name repeats, and only erroring when a second different reader is offered for the same name, would fix the real ambiguity without breaking these callers.

[ingvarch]

Fixed. The form now tracks name -> reader. A repeated name with the same reader, or with no reader at all, reuses the part already written; only a second different reader errors. Sticker sets included.

One correction: your second snippet (second entry with no reader) failed on main too, with nil attachment for attach://p.jpg from #296, so it is not a regression from this PR. It works now, because the nil check only applies when no part of that name exists yet.

[negasus]

Minor: requestForm only intercepts CreateFormFile, so the invariant the README now states ("part names must be unique within one request") is only half enforced — a file part and a form field can still collide:

b.EditMessageMedia(ctx, &bot.EditMessageMediaParams{
    Media: &models.InputMediaVideo{Media: "attach://media", MediaAttachment: r},
})
// builds fine, emits two parts both named "media" (one file, one field)

That's the exact ambiguity the check was added to prevent. Tracking CreateFormField names in the same map closes it.

[ingvarch]

Fixed. Dropped the embedded formWriter so the wrapper cannot be bypassed, and added addFieldPart. File parts and form fields now share one name space, so your EditMessageMedia example errors.

[negasus]

Minor: this trades the panic for a server-side rejection rather than a working call. The field is skipped for upload, but the encoder still emits "thumbnail":null:

{"type":"video","media":"file_id","thumbnail":null}

and Telegram rejects a null where it expects a String. The caller's intent with a nil thumbnail is clearly "no thumbnail", so omitting the key entirely would let the call succeed. Same for InputRichMessageMedia.Media → "media":null.

[ingvarch]

Fixed for the thumbnail: typed nils are normalised at marshal time, so omitempty drops the key.

Went the other way on InputRichMessageMedia.Media, which now errors. media is required there, so omitting the key still leaves an invalid entry, and it is the same class as the nil media-slice item above. Say the word if you would rather have it omitted.

[negasus]

Minor: upload.Data is validated just above but upload.Filename isn't. With Thumbnail: &models.InputFileUpload{Data: r} (no filename) the form gets a part named "" and the media JSON gets "thumbnail":"attach://". Telegram answers with an opaque Bad Request instead of the library catching it, which is inconsistent with the nil-Data check right above.

[ingvarch]

Fixed. An empty Filename on a nested upload errors: empty filename for nested upload, it is the attach:// reference.

[negasus]

Minor, but worth doing while this comment is being written: the JSON string is built by concatenation with no escaping. Now that Filename doubles as the attach:// reference and the part name, a filename containing " or \ produces invalid JSON and the request fails late — after the file part has already been streamed into the pipe:

&models.InputFileUpload{Filename: "a\"b\\c.jpg", Data: r}
// json: error calling MarshalJSON for type *models.InputFileUpload:
// invalid character 'b' after top-level value

return json.Marshal("attach://" + i.Filename)

InputFileString.MarshalJSON below has the identical flaw for a file_id/URL containing a quote.

[ingvarch]

Fixed. Both use json.Marshal now.

[negasus]

Re-checked all seven points on b85209e, against a main worktree for comparison. Everything I raised is fixed, and I verified each one by building the form rather than by reading the diff:

• typed nil *InputFileUpload in a top level InputFile field → nil value for field thumbnail. Worth naming: this one panicked on main, so the PR closes a pre-existing crash, not one it introduced.
• typed nil in []InputMedia, in a single InputMedia field, and in InputRichMessage.Media → all three error, none panic.
• duplicate part names: the same reader, or none, writes one part and reuses it with the right content; a second different reader errors. And your correction stands — {reader}, {no reader} errors on main too, so that was not a regression from this branch. My mistake.
• file part / form field name space is now shared: attach://media errors in both editMessageMedia and sendMediaGroup. On main that silently emitted two parts named media.
• a nil thumbnail is omitted rather than sent as null, for InputMedia* and InputPaidMediaVideo, including a typed nil *InputFileString.
• an empty nested Filename errors; MarshalJSON escapes properly on both types.

The core fix does what it says: nested rich-message media and container blocks (checked Collage → Photo) write their file parts, and a nested thumbnail is uploaded and referenced as attach://thumb.jpg where main emitted "thumbnail":"@thumb.jpg" with no part at all. Block coverage is complete — List, BlockQuotation, Collage, Slideshow, Details are the only variants nesting []InputRichBlock, and all six media blocks are walked. All four params fields are wired, and the two pointer ones are omitempty, so a nil never reaches the walker. go vet, gofmt and go test -race ./... are clean, and no existing test assertion was weakened.

On InputRichMessageMedia.Media: erroring is the right call, media is required there, so omitting the key would only produce an invalid entry.

Merging. Five notes below, none blocking, all fine as a follow-up:

1. sameReader requires a comparable dynamic type. A reader that is a value of a non-comparable struct type, used twice under one name, now errors duplicate form part name where main accepted it. That is a false positive rather than a real ambiguity — narrow, since nearly every reader is a pointer, but reflect.Type.Comparable() fails into "different" silently.

2. One reader used under two different part names still writes the second part empty, because the reader is already drained, and says nothing. Not a regression, but it is the same class of silent corruption this PR closes, and the fileParts map now makes it detectable.

3. Order dependence: [{no reader}, {reader}] under one name errors, while the reverse succeeds. Same on main; a line in the README or a pre-scan would settle it.

4. A Filename containing CR or LF diverges — mime/multipart percent-escapes the part name (a%0Ab.jpg) while the JSON keeps the literal, so the reference can never resolve. " and \ round-trip correctly. Very much an edge case; rejecting control characters in a nested Filename would close it.

5. The CHANGELOG doesn't flag the file-part/form-field collision as breaking. attach://media is a natural name and now hard-errors on sendMediaGroup, editMessageMedia and sendPaidMedia. It was already broken against Telegram, but a line saying so would help anyone whose code stops building a request.

Thanks for the thorough turnaround on this.