Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 48 additions & 0 deletions shortcuts/im/coverage_additional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,3 +602,51 @@ func TestMediaBufferReader(t *testing.T) {
}
}
}

func TestMediaBufferFileName(t *testing.T) {
tests := []struct {
label string
buf mediaBuffer
want string
}{
{"original URL filename", mediaBuffer{name: "report.pdf", ext: ".pdf"}, "report.pdf"},
{"name with spaces", mediaBuffer{name: "Q1 report.pdf", ext: ".pdf"}, "Q1 report.pdf"},
{"download fallback", mediaBuffer{name: "download", ext: ""}, "download"},
{"ext not leaked into name", mediaBuffer{name: "x", ext: ".mp4"}, "x"},
}
for _, tt := range tests {
t.Run(tt.label, func(t *testing.T) {
if got := tt.buf.FileName(); got != tt.want {
t.Fatalf("FileName() = %q, want %q", got, tt.want)
}
})
}
}

// TestNewMediaBufferFromBytesURLFilename locks in the URL -> mediaBuffer.name
// wiring so a future refactor cannot regress back to the "media.<ext>" synthetic
// filename that was shipped in 91067ec.
func TestNewMediaBufferFromBytesURLFilename(t *testing.T) {
tests := []struct {
label string
url string
want string
}{
{"path filename", "http://example.com/report.pdf", "report.pdf"},
{"filename survives query string", "http://example.com/videos/clip.mp4?token=abc", "clip.mp4"},
{"percent-encoded spaces decoded", "http://example.com/Q1%20report.pdf", "Q1 report.pdf"},
{"no path falls back to download", "http://example.com/", "download"},
{"non-http scheme falls back to download", "ftp://example.com/x.pdf", "download"},
}
for _, tt := range tests {
t.Run(tt.label, func(t *testing.T) {
mb := newMediaBufferFromBytes([]byte("payload"), ".pdf", tt.url)
if got := mb.FileName(); got != tt.want {
t.Fatalf("FileName() for %q = %q, want %q", tt.url, got, tt.want)
}
if got := mb.FileName(); strings.HasPrefix(got, "media") && tt.want != "media" {
t.Fatalf("regression: FileName() returned synthetic %q for %q", got, tt.url)
}
})
}
}
14 changes: 11 additions & 3 deletions shortcuts/im/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@
type mediaBuffer struct {
data []byte
ext string // file extension including leading dot, e.g. ".mp4"
name string // original file name extracted from the source URL
}

// newMediaBuffer downloads URL content into memory via downloadURLToReader.
Expand All @@ -598,7 +599,14 @@
if err != nil {
return nil, fmt.Errorf("download failed: %w", err)
}
return &mediaBuffer{data: data, ext: ext}, nil
return newMediaBufferFromBytes(data, ext, rawURL), nil

Check warning on line 602 in shortcuts/im/helpers.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/im/helpers.go#L602

Added line #L602 was not covered by tests
}

// newMediaBufferFromBytes builds a mediaBuffer from already-downloaded bytes.
// Split out from newMediaBuffer so the URL-to-filename wiring is testable
// without going through the hardened download transport.
func newMediaBufferFromBytes(data []byte, ext, rawURL string) *mediaBuffer {
return &mediaBuffer{data: data, ext: ext, name: fileNameFromURL(rawURL)}
}

// Reader returns a new io.Reader over the buffered data. Each call returns a
Expand All @@ -608,9 +616,9 @@
return bytes.NewReader(b.data)
}

// FileName returns a synthetic file name based on the URL extension.
// FileName returns the original file name extracted from the source URL.
func (b *mediaBuffer) FileName() string {
return "media" + b.ext
return b.name
}

// FileType returns the IM file type detected from the extension.
Expand Down
35 changes: 35 additions & 0 deletions shortcuts/im/helpers_network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -892,3 +892,38 @@ func TestResolveLocalMediaFile(t *testing.T) {
t.Fatalf("resolveLocalMedia(file) = %q, want %q", got, "file_via_resolve")
}
}

// TestUploadFileToIMPreservesLocalFileName locks in that local uploads keep
// the basename of the caller-supplied path as the multipart file_name, so the
// URL-side fix for mediaBuffer cannot silently regress the local branch later.
func TestUploadFileToIMPreservesLocalFileName(t *testing.T) {
var gotBody string
runtime := newBotShortcutRuntime(t, shortcutRoundTripFunc(func(req *http.Request) (*http.Response, error) {
if strings.Contains(req.URL.Path, "/open-apis/im/v1/files") {
body, err := io.ReadAll(req.Body)
if err != nil {
return nil, err
}
gotBody = string(body)
return shortcutJSONResponse(200, map[string]interface{}{
"code": 0,
"data": map[string]interface{}{"file_key": "file_uploaded"},
}), nil
}
return nil, fmt.Errorf("unexpected request: %s", req.URL.String())
}))

cmdutil.TestChdir(t, t.TempDir())

localName := "Q1-meeting-notes.pdf"
if err := os.WriteFile(localName, []byte("pdfdata"), 0600); err != nil {
t.Fatalf("WriteFile() error = %v", err)
}

if _, err := uploadFileToIM(context.Background(), runtime, "./"+localName, "pdf", ""); err != nil {
t.Fatalf("uploadFileToIM() error = %v", err)
}
if !strings.Contains(gotBody, `name="file_name"`) || !strings.Contains(gotBody, localName) {
t.Fatalf("upload body missing local filename %q; got: %q", localName, gotBody)
}
}
Loading