Skip to content

Commit eda2b9c

Browse files
authored
revert: undo auto-resolve local image paths in draft body HTML (#199)
* Revert "fix(mail): clarify that file path flags only accept relative paths (#141)" This reverts commit 1ffe870. * Revert "feat(mail): auto-resolve local image paths in draft body HTML (#81) (#139)" This reverts commit 70c72a2. * Reapply "fix(mail): clarify that file path flags only accept relative paths (#141)" This reverts commit d465e08.
1 parent a703202 commit eda2b9c

File tree

5 files changed

+77
-969
lines changed

5 files changed

+77
-969
lines changed

shortcuts/mail/draft/patch.go

Lines changed: 53 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,12 @@ import (
88
"mime"
99
"os"
1010
"path/filepath"
11-
"regexp"
1211
"strings"
1312

14-
"github.com/google/uuid"
1513
"github.com/larksuite/cli/internal/validate"
1614
"github.com/larksuite/cli/shortcuts/mail/filecheck"
1715
)
1816

19-
// imgSrcRegexp matches <img ... src="value" ...> and captures the src value.
20-
// It handles both single and double quotes.
21-
var imgSrcRegexp = regexp.MustCompile(`(?i)<img\s(?:[^>]*?\s)?src\s*=\s*["']([^"']+)["']`)
22-
2317
var protectedHeaders = map[string]bool{
2418
"message-id": true,
2519
"mime-version": true,
@@ -39,10 +33,13 @@ func Apply(snapshot *DraftSnapshot, patch Patch) error {
3933
return err
4034
}
4135
}
42-
if err := postProcessInlineImages(snapshot); err != nil {
36+
if err := refreshSnapshot(snapshot); err != nil {
37+
return err
38+
}
39+
if err := validateInlineCIDAfterApply(snapshot); err != nil {
4340
return err
4441
}
45-
return refreshSnapshot(snapshot)
42+
return validateOrphanedInlineCIDAfterApply(snapshot)
4643
}
4744

4845
func applyOp(snapshot *DraftSnapshot, op PatchOp, options PatchOptions) error {
@@ -526,56 +523,45 @@ func addAttachment(snapshot *DraftSnapshot, path string) error {
526523
return nil
527524
}
528525

529-
// loadAndAttachInline reads a local image file, validates its format,
530-
// creates a MIME inline part, and attaches it to the snapshot's
531-
// multipart/related container. If container is non-nil it is reused;
532-
// otherwise the container is resolved from the snapshot.
533-
func loadAndAttachInline(snapshot *DraftSnapshot, path, cid, fileName string, container *Part) (*Part, error) {
526+
func addInline(snapshot *DraftSnapshot, path, cid, fileName, contentType string) error {
534527
safePath, err := validate.SafeInputPath(path)
535528
if err != nil {
536-
return nil, fmt.Errorf("inline image %q: %w", path, err)
529+
return fmt.Errorf("inline image %q: %w", path, err)
537530
}
538531
info, err := os.Stat(safePath)
539532
if err != nil {
540-
return nil, fmt.Errorf("inline image %q: %w", path, err)
533+
return err
541534
}
542535
if err := checkSnapshotAttachmentLimit(snapshot, info.Size(), nil); err != nil {
543-
return nil, err
536+
return err
544537
}
545538
content, err := os.ReadFile(safePath)
546539
if err != nil {
547-
return nil, fmt.Errorf("inline image %q: %w", path, err)
540+
return err
548541
}
549542
name := fileName
550543
if strings.TrimSpace(name) == "" {
551544
name = filepath.Base(path)
552545
}
553546
detectedCT, err := filecheck.CheckInlineImageFormat(name, content)
554547
if err != nil {
555-
return nil, fmt.Errorf("inline image %q: %w", path, err)
548+
return err
556549
}
557-
inline, err := newInlinePart(safePath, content, cid, name, detectedCT)
550+
inline, err := newInlinePart(path, content, cid, fileName, detectedCT)
558551
if err != nil {
559-
return nil, fmt.Errorf("inline image %q: %w", path, err)
552+
return err
560553
}
561-
if container == nil {
562-
containerRef := primaryBodyRootRef(&snapshot.Body)
563-
if containerRef == nil || *containerRef == nil {
564-
return nil, fmt.Errorf("draft has no primary body container")
565-
}
566-
container, err = ensureInlineContainerRef(containerRef)
567-
if err != nil {
568-
return nil, fmt.Errorf("inline image %q: %w", path, err)
569-
}
554+
containerRef := primaryBodyRootRef(&snapshot.Body)
555+
if containerRef == nil || *containerRef == nil {
556+
return fmt.Errorf("draft has no primary body container")
557+
}
558+
container, err := ensureInlineContainerRef(containerRef)
559+
if err != nil {
560+
return err
570561
}
571562
container.Children = append(container.Children, inline)
572563
container.Dirty = true
573-
return container, nil
574-
}
575-
576-
func addInline(snapshot *DraftSnapshot, path, cid, fileName, contentType string) error {
577-
_, err := loadAndAttachInline(snapshot, path, cid, fileName, nil)
578-
return err
564+
return nil
579565
}
580566

581567
func replaceInline(snapshot *DraftSnapshot, partID, path, cid, fileName, contentType string) error {
@@ -776,9 +762,6 @@ func newInlinePart(path string, content []byte, cid, fileName, contentType strin
776762
if err := validate.RejectCRLF(cid, "inline cid"); err != nil {
777763
return nil, err
778764
}
779-
if strings.ContainsAny(cid, " \t<>()") {
780-
return nil, fmt.Errorf("inline cid %q contains invalid characters (spaces, tabs, angle brackets, or parentheses are not allowed)", cid)
781-
}
782765
if err := validate.RejectCRLF(fileName, "inline filename"); err != nil {
783766
return nil, err
784767
}
@@ -874,152 +857,59 @@ func removeHeader(headers *[]Header, name string) {
874857
*headers = next
875858
}
876859

877-
// uriSchemeRegexp matches a URI scheme (RFC 3986: ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) ":").
878-
var uriSchemeRegexp = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.\-]*:`)
879-
880-
// isLocalFileSrc returns true if src is a local file path.
881-
// Any URI with a scheme (http:, cid:, data:, ftp:, blob:, file:, etc.)
882-
// or protocol-relative URL (//host/...) is rejected.
883-
func isLocalFileSrc(src string) bool {
884-
trimmed := strings.TrimSpace(src)
885-
if trimmed == "" {
886-
return false
887-
}
888-
if strings.HasPrefix(trimmed, "//") {
889-
return false
890-
}
891-
return !uriSchemeRegexp.MatchString(trimmed)
892-
}
893-
894-
// generateCID returns a random UUID string suitable for use as a Content-ID.
895-
// UUIDs contain only [0-9a-f-], which is inherently RFC-safe and unique,
896-
// avoiding all filename-derived encoding/collision issues.
897-
func generateCID() (string, error) {
898-
id, err := uuid.NewRandom()
899-
if err != nil {
900-
return "", fmt.Errorf("failed to generate CID: %w", err)
860+
// validateInlineCIDAfterApply checks that all CID references in the HTML body
861+
// resolve to actual inline MIME parts. This is called after Apply (editing) to
862+
// prevent broken CID references, but NOT during Parse (where broken CIDs
863+
// should not block opening the draft).
864+
func validateInlineCIDAfterApply(snapshot *DraftSnapshot) error {
865+
htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID)
866+
if htmlPart == nil {
867+
return nil
901868
}
902-
return id.String(), nil
903-
}
904-
905-
// resolveLocalImgSrc scans HTML for <img src="local/path"> references,
906-
// creates MIME inline parts for each local file, and returns the HTML
907-
// with those src attributes replaced by cid: URIs.
908-
func resolveLocalImgSrc(snapshot *DraftSnapshot, html string) (string, error) {
909-
matches := imgSrcRegexp.FindAllStringSubmatchIndex(html, -1)
910-
if len(matches) == 0 {
911-
return html, nil
869+
refs := extractCIDRefs(string(htmlPart.Body))
870+
if len(refs) == 0 {
871+
return nil
912872
}
913-
914-
var container *Part
915-
// Cache resolved paths so the same file is only attached once.
916-
pathToCID := make(map[string]string)
917-
918-
// Iterate in reverse so that index offsets remain valid after replacement.
919-
for i := len(matches) - 1; i >= 0; i-- {
920-
srcStart, srcEnd := matches[i][2], matches[i][3]
921-
src := html[srcStart:srcEnd]
922-
if !isLocalFileSrc(src) {
873+
cids := make(map[string]bool)
874+
for _, part := range flattenParts(snapshot.Body) {
875+
if part == nil || part.ContentID == "" {
923876
continue
924877
}
925-
926-
resolvedPath, err := validate.SafeInputPath(src)
927-
if err != nil {
928-
return "", fmt.Errorf("inline image %q: %w", src, err)
929-
}
930-
931-
cid, ok := pathToCID[resolvedPath]
932-
if !ok {
933-
fileName := filepath.Base(src)
934-
cid, err = generateCID()
935-
if err != nil {
936-
return "", err
937-
}
938-
pathToCID[resolvedPath] = cid
939-
940-
container, err = loadAndAttachInline(snapshot, src, cid, fileName, container)
941-
if err != nil {
942-
return "", err
943-
}
944-
}
945-
946-
html = html[:srcStart] + "cid:" + cid + html[srcEnd:]
947-
}
948-
949-
return html, nil
950-
}
951-
952-
// removeOrphanedInlineParts removes inline MIME parts whose ContentID
953-
// is not in the referencedCIDs set from all multipart/related containers.
954-
func removeOrphanedInlineParts(root *Part, referencedCIDs map[string]bool) {
955-
if root == nil {
956-
return
957-
}
958-
if !strings.EqualFold(root.MediaType, "multipart/related") {
959-
for _, child := range root.Children {
960-
removeOrphanedInlineParts(child, referencedCIDs)
961-
}
962-
return
878+
cids[strings.ToLower(part.ContentID)] = true
963879
}
964-
kept := make([]*Part, 0, len(root.Children))
965-
for _, child := range root.Children {
966-
if child == nil {
967-
continue
968-
}
969-
if strings.EqualFold(child.ContentDisposition, "inline") && child.ContentID != "" {
970-
if !referencedCIDs[strings.ToLower(child.ContentID)] {
971-
root.Dirty = true
972-
continue
973-
}
880+
for _, ref := range refs {
881+
if !cids[strings.ToLower(ref)] {
882+
return fmt.Errorf("html body references missing inline cid %q", ref)
974883
}
975-
kept = append(kept, child)
976884
}
977-
root.Children = kept
885+
return nil
978886
}
979887

980-
// postProcessInlineImages is the unified post-processing step that:
981-
// 1. Resolves local <img src="./path"> to inline CID parts.
982-
// 2. Validates all CID references in HTML resolve to MIME parts.
983-
// 3. Removes orphaned inline MIME parts no longer referenced by HTML.
984-
func postProcessInlineImages(snapshot *DraftSnapshot) error {
985-
htmlPart := findPrimaryBodyPart(snapshot.Body, "text/html")
888+
// validateOrphanedInlineCIDAfterApply checks the reverse direction: every
889+
// inline MIME part with a ContentID must be referenced by the HTML body.
890+
// An orphaned inline part (CID exists but HTML has no <img src="cid:...">) will
891+
// be displayed as an unexpected attachment by most mail clients.
892+
func validateOrphanedInlineCIDAfterApply(snapshot *DraftSnapshot) error {
893+
htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID)
986894
if htmlPart == nil {
987895
return nil
988896
}
989-
990-
origHTML := string(htmlPart.Body)
991-
// Note: if resolveLocalImgSrc returns an error after partially attaching
992-
// inline parts to the snapshot, those parts are orphaned but will be
993-
// cleaned up by removeOrphanedInlineParts on the next successful Apply.
994-
html, err := resolveLocalImgSrc(snapshot, origHTML)
995-
if err != nil {
996-
return err
997-
}
998-
if html != origHTML {
999-
htmlPart.Body = []byte(html)
1000-
htmlPart.Dirty = true
1001-
}
1002-
1003-
refs := extractCIDRefs(html)
897+
refs := extractCIDRefs(string(htmlPart.Body))
1004898
refSet := make(map[string]bool, len(refs))
1005899
for _, ref := range refs {
1006900
refSet[strings.ToLower(ref)] = true
1007901
}
1008-
1009-
cidParts := make(map[string]bool)
902+
var orphaned []string
1010903
for _, part := range flattenParts(snapshot.Body) {
1011904
if part == nil || part.ContentID == "" {
1012905
continue
1013906
}
1014-
cidParts[strings.ToLower(part.ContentID)] = true
1015-
}
1016-
1017-
for _, ref := range refs {
1018-
if !cidParts[strings.ToLower(ref)] {
1019-
return fmt.Errorf("html body references missing inline cid %q", ref)
907+
if !refSet[strings.ToLower(part.ContentID)] {
908+
orphaned = append(orphaned, part.ContentID)
1020909
}
1021910
}
1022-
1023-
removeOrphanedInlineParts(snapshot.Body, refSet)
911+
if len(orphaned) > 0 {
912+
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)
913+
}
1024914
return nil
1025915
}

0 commit comments

Comments
 (0)