Skip to content

revert: undo auto-resolve local image paths in draft body HTML #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
infeng merged 3 commits into from
Apr 1, 2026
Merged

revert: undo auto-resolve local image paths in draft body HTML #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d465e08
Revert "fix(mail): clarify that file path flags only accept relative …
infeng Apr 1, 2026
b21a90f
Revert "feat(mail): auto-resolve local image paths in draft body HTML…
infeng Apr 1, 2026
5051e25
Reapply "fix(mail): clarify that file path flags only accept relative…
infeng Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
216 changes: 53 additions & 163 deletions shortcuts/mail/draft/patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,12 @@ import (
"mime"
"os"
"path/filepath"
"regexp"
"strings"

"github.com/google/uuid"
"github.com/larksuite/cli/internal/validate"
"github.com/larksuite/cli/shortcuts/mail/filecheck"
)

// imgSrcRegexp matches <img ... src="value" ...> and captures the src value.
// It handles both single and double quotes.
var imgSrcRegexp = regexp.MustCompile(`(?i)<img\s(?:[^>]*?\s)?src\s*=\s*["']([^"']+)["']`)

var protectedHeaders = map[string]bool{
"message-id": true,
"mime-version": true,
Expand All @@ -39,10 +33,13 @@ func Apply(snapshot *DraftSnapshot, patch Patch) error {
return err
}
}
if err := postProcessInlineImages(snapshot); err != nil {
if err := refreshSnapshot(snapshot); err != nil {
return err
}
if err := validateInlineCIDAfterApply(snapshot); err != nil {
return err
}
return refreshSnapshot(snapshot)
return validateOrphanedInlineCIDAfterApply(snapshot)
}

func applyOp(snapshot *DraftSnapshot, op PatchOp, options PatchOptions) error {
Expand Down Expand Up @@ -526,56 +523,45 @@ func addAttachment(snapshot *DraftSnapshot, path string) error {
return nil
}

// loadAndAttachInline reads a local image file, validates its format,
// creates a MIME inline part, and attaches it to the snapshot's
// multipart/related container. If container is non-nil it is reused;
// otherwise the container is resolved from the snapshot.
func loadAndAttachInline(snapshot *DraftSnapshot, path, cid, fileName string, container *Part) (*Part, error) {
func addInline(snapshot *DraftSnapshot, path, cid, fileName, contentType string) error {
safePath, err := validate.SafeInputPath(path)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
return fmt.Errorf("inline image %q: %w", path, err)
}
info, err := os.Stat(safePath)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
return err
}
if err := checkSnapshotAttachmentLimit(snapshot, info.Size(), nil); err != nil {
return nil, err
return err
}
content, err := os.ReadFile(safePath)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
return err
}
name := fileName
if strings.TrimSpace(name) == "" {
name = filepath.Base(path)
}
detectedCT, err := filecheck.CheckInlineImageFormat(name, content)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
return err
}
inline, err := newInlinePart(safePath, content, cid, name, detectedCT)
inline, err := newInlinePart(path, content, cid, fileName, detectedCT)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
return err
}
if container == nil {
containerRef := primaryBodyRootRef(&snapshot.Body)
if containerRef == nil || *containerRef == nil {
return nil, fmt.Errorf("draft has no primary body container")
}
container, err = ensureInlineContainerRef(containerRef)
if err != nil {
return nil, fmt.Errorf("inline image %q: %w", path, err)
}
containerRef := primaryBodyRootRef(&snapshot.Body)
if containerRef == nil || *containerRef == nil {
return fmt.Errorf("draft has no primary body container")
}
container, err := ensureInlineContainerRef(containerRef)
if err != nil {
return err
}
container.Children = append(container.Children, inline)
container.Dirty = true
return container, nil
}

func addInline(snapshot *DraftSnapshot, path, cid, fileName, contentType string) error {
_, err := loadAndAttachInline(snapshot, path, cid, fileName, nil)
return err
return nil
}

func replaceInline(snapshot *DraftSnapshot, partID, path, cid, fileName, contentType string) error {
Expand Down Expand Up @@ -776,9 +762,6 @@ func newInlinePart(path string, content []byte, cid, fileName, contentType strin
if err := validate.RejectCRLF(cid, "inline cid"); err != nil {
return nil, err
}
Comment thread
infeng marked this conversation as resolved.
if strings.ContainsAny(cid, " \t<>()") {
return nil, fmt.Errorf("inline cid %q contains invalid characters (spaces, tabs, angle brackets, or parentheses are not allowed)", cid)
}
if err := validate.RejectCRLF(fileName, "inline filename"); err != nil {
return nil, err
}
Expand Down Expand Up @@ -874,152 +857,59 @@ func removeHeader(headers *[]Header, name string) {
*headers = next
}

// uriSchemeRegexp matches a URI scheme (RFC 3986: ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) ":").
var uriSchemeRegexp = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.\-]*:`)

// isLocalFileSrc returns true if src is a local file path.
// Any URI with a scheme (http:, cid:, data:, ftp:, blob:, file:, etc.)
// or protocol-relative URL (//host/...) is rejected.
func isLocalFileSrc(src string) bool {
trimmed := strings.TrimSpace(src)
if trimmed == "" {
return false
}
if strings.HasPrefix(trimmed, "//") {
return false
}
return !uriSchemeRegexp.MatchString(trimmed)
}

// generateCID returns a random UUID string suitable for use as a Content-ID.
// UUIDs contain only [0-9a-f-], which is inherently RFC-safe and unique,
// avoiding all filename-derived encoding/collision issues.
func generateCID() (string, error) {
id, err := uuid.NewRandom()
if err != nil {
return "", fmt.Errorf("failed to generate CID: %w", err)
// validateInlineCIDAfterApply checks that all CID references in the HTML body
// resolve to actual inline MIME parts. This is called after Apply (editing) to
// prevent broken CID references, but NOT during Parse (where broken CIDs
// should not block opening the draft).
func validateInlineCIDAfterApply(snapshot *DraftSnapshot) error {
htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID)
if htmlPart == nil {
return nil
}
return id.String(), nil
}

// resolveLocalImgSrc scans HTML for <img src="local/path"> references,
// creates MIME inline parts for each local file, and returns the HTML
// with those src attributes replaced by cid: URIs.
func resolveLocalImgSrc(snapshot *DraftSnapshot, html string) (string, error) {
matches := imgSrcRegexp.FindAllStringSubmatchIndex(html, -1)
if len(matches) == 0 {
return html, nil
refs := extractCIDRefs(string(htmlPart.Body))
if len(refs) == 0 {
return nil
}

var container *Part
// Cache resolved paths so the same file is only attached once.
pathToCID := make(map[string]string)

// Iterate in reverse so that index offsets remain valid after replacement.
for i := len(matches) - 1; i >= 0; i-- {
srcStart, srcEnd := matches[i][2], matches[i][3]
src := html[srcStart:srcEnd]
if !isLocalFileSrc(src) {
cids := make(map[string]bool)
for _, part := range flattenParts(snapshot.Body) {
if part == nil || part.ContentID == "" {
continue
}

resolvedPath, err := validate.SafeInputPath(src)
if err != nil {
return "", fmt.Errorf("inline image %q: %w", src, err)
}

cid, ok := pathToCID[resolvedPath]
if !ok {
fileName := filepath.Base(src)
cid, err = generateCID()
if err != nil {
return "", err
}
pathToCID[resolvedPath] = cid

container, err = loadAndAttachInline(snapshot, src, cid, fileName, container)
if err != nil {
return "", err
}
}

html = html[:srcStart] + "cid:" + cid + html[srcEnd:]
}

return html, nil
}

// removeOrphanedInlineParts removes inline MIME parts whose ContentID
// is not in the referencedCIDs set from all multipart/related containers.
func removeOrphanedInlineParts(root *Part, referencedCIDs map[string]bool) {
if root == nil {
return
}
if !strings.EqualFold(root.MediaType, "multipart/related") {
for _, child := range root.Children {
removeOrphanedInlineParts(child, referencedCIDs)
}
return
cids[strings.ToLower(part.ContentID)] = true
}
kept := make([]*Part, 0, len(root.Children))
for _, child := range root.Children {
if child == nil {
continue
}
if strings.EqualFold(child.ContentDisposition, "inline") && child.ContentID != "" {
if !referencedCIDs[strings.ToLower(child.ContentID)] {
root.Dirty = true
continue
}
for _, ref := range refs {
if !cids[strings.ToLower(ref)] {
return fmt.Errorf("html body references missing inline cid %q", ref)
}
kept = append(kept, child)
}
root.Children = kept
return nil
}

// postProcessInlineImages is the unified post-processing step that:
// 1. Resolves local <img src="./path"> to inline CID parts.
// 2. Validates all CID references in HTML resolve to MIME parts.
// 3. Removes orphaned inline MIME parts no longer referenced by HTML.
func postProcessInlineImages(snapshot *DraftSnapshot) error {
htmlPart := findPrimaryBodyPart(snapshot.Body, "text/html")
// validateOrphanedInlineCIDAfterApply checks the reverse direction: every
// inline MIME part with a ContentID must be referenced by the HTML body.
// An orphaned inline part (CID exists but HTML has no <img src="cid:...">) will
// be displayed as an unexpected attachment by most mail clients.
func validateOrphanedInlineCIDAfterApply(snapshot *DraftSnapshot) error {
htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID)
if htmlPart == nil {
return nil
}

origHTML := string(htmlPart.Body)
// Note: if resolveLocalImgSrc returns an error after partially attaching
// inline parts to the snapshot, those parts are orphaned but will be
// cleaned up by removeOrphanedInlineParts on the next successful Apply.
html, err := resolveLocalImgSrc(snapshot, origHTML)
if err != nil {
return err
}
if html != origHTML {
htmlPart.Body = []byte(html)
htmlPart.Dirty = true
}

refs := extractCIDRefs(html)
refs := extractCIDRefs(string(htmlPart.Body))
refSet := make(map[string]bool, len(refs))
for _, ref := range refs {
refSet[strings.ToLower(ref)] = true
}

cidParts := make(map[string]bool)
var orphaned []string
for _, part := range flattenParts(snapshot.Body) {
if part == nil || part.ContentID == "" {
continue
}
cidParts[strings.ToLower(part.ContentID)] = true
}

for _, ref := range refs {
if !cidParts[strings.ToLower(ref)] {
return fmt.Errorf("html body references missing inline cid %q", ref)
if !refSet[strings.ToLower(part.ContentID)] {
orphaned = append(orphaned, part.ContentID)
}
}
Comment thread
infeng marked this conversation as resolved.

removeOrphanedInlineParts(snapshot.Body, refSet)
if len(orphaned) > 0 {
return fmt.Errorf("inline MIME parts have no <img> reference in the HTML body and will appear as unexpected attachments: orphaned cids %v; if you used set_body, make sure the new body preserves all existing cid:... references", orphaned)
}
return nil
}
Comment thread
infeng marked this conversation as resolved.
Loading
Loading