Skip to content

Commit b3be68a

Browse files
jlianjlian
committed
fix(msteams): fix inline pasted image downloads (PR #10902 rebase)
1 parent 859889a commit b3be68a

File tree

6 files changed

+227
-57
lines changed

6 files changed

+227
-57
lines changed

‎extensions/msteams/src/attachments.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,9 @@ const createMediaEntriesWithType = (contentType: string, ...paths: string[]) =>
212212
paths.map((path) => ({ path, contentType }));
213213
const createHostedContentsWithType = (contentType: string, ...ids: string[]) =>
214214
ids.map((id) => ({ id, contentType, contentBytes: PNG_BASE64 }));
215+
/** Like createHostedContentsWithType but with contentBytes: null to exercise the $value fetch path. */
216+
const createHostedContentsNullBytes = (contentType: string, ...ids: string[]) =>
217+
ids.map((id) => ({ id, contentType, contentBytes: null }));
215218
const createImageMediaEntries = (...paths: string[]) =>
216219
createMediaEntriesWithType(CONTENT_TYPE_IMAGE_PNG, ...paths);
217220
const createHostedImageContents = (...ids: string[]) =>
@@ -848,6 +851,20 @@ describe("msteams attachments", () => {
848851
expectAttachmentMediaLength(media, 0);
849852
expect(fetchMock).toHaveBeenCalledTimes(1);
850853
});
854+
855+
it("appends /views/original to Bot Framework attachment URLs", async () => {
856+
const bfUrl = "https://smba.trafficmanager.net/amer/72f988bf/v3/attachments/0-wus-d11-abc";
857+
const media = await downloadAttachmentsWithFetch(
858+
createContentUrlAttachments("image/*", bfUrl),
859+
fetchRemoteMediaMock,
860+
{ allowHosts: ["smba.trafficmanager.net"], resolveFn: publicResolveFn },
861+
);
862+
863+
expect(fetchRemoteMediaMock).toHaveBeenCalledWith(
864+
expect.objectContaining({ url: `${bfUrl}/views/original` }),
865+
);
866+
expectAttachmentMediaLength(media, 1);
867+
});
851868
});
852869

853870
describe("buildMSTeamsGraphMessageUrls", () => {
@@ -927,6 +944,41 @@ describe("msteams attachments", () => {
927944
expect(calledUrls.some((url) => url.startsWith(GRAPH_SHARES_URL_PREFIX))).toBe(true);
928945
expect(calledUrls).not.toContain(escapedUrl);
929946
});
947+
948+
it("skips hosted content when $value fetch fails", async () => {
949+
const tokenProvider = createTokenProvider();
950+
const graphFetchMock = vi.fn(async (url: string) => {
951+
if (url.endsWith("/hostedContents")) {
952+
// contentBytes: null forces the $value fetch path
953+
return createGraphCollectionResponse(
954+
createHostedContentsNullBytes(CONTENT_TYPE_IMAGE_PNG, "1"),
955+
);
956+
}
957+
if (url.includes("/hostedContents/1/$value")) {
958+
return createNotFoundResponse();
959+
}
960+
if (url.endsWith("/attachments")) {
961+
return createGraphCollectionResponse([]);
962+
}
963+
return createNotFoundResponse();
964+
}) as unknown as FetchFn;
965+
966+
const { media } = await downloadGraphMediaWithMockOptions(
967+
{},
968+
{
969+
fetchFn: graphFetchMock,
970+
tokenProvider,
971+
},
972+
);
973+
974+
expectAttachmentMediaLength(media.media, 0);
975+
expect(saveMediaBufferMock).not.toHaveBeenCalled();
976+
// Verify the $value fetch was actually attempted with the correct URL.
977+
expect(graphFetchMock).toHaveBeenCalledWith(
978+
expect.stringContaining("/hostedContents/1/$value"),
979+
expect.anything(),
980+
);
981+
});
930982
});
931983

932984
describe("buildMSTeamsMediaPayload", () => {

‎extensions/msteams/src/attachments.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ export {
33
/** @deprecated Use `downloadMSTeamsAttachments` instead. */
44
downloadMSTeamsImageAttachments,
55
} from "./attachments/download.js";
6-
export { buildMSTeamsGraphMessageUrls, downloadMSTeamsGraphMedia } from "./attachments/graph.js";
6+
export {
7+
buildMSTeamsGraphMessageUrls,
8+
downloadGraphHostedContent,
9+
downloadMSTeamsGraphMedia,
10+
} from "./attachments/graph.js";
711
export {
812
buildMSTeamsAttachmentPlaceholder,
913
summarizeMSTeamsHtmlAttachments,

‎extensions/msteams/src/attachments/download.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@ type DownloadCandidate = {
2626
placeholder: string;
2727
};
2828

29+
/** Detect Bot Framework /v3/attachments/{id} URLs that need /views/original. */
30+
const BF_ATTACHMENT_RE = /\/v3\/attachments\/[^/]+\/?$/i;
31+
function isBotFrameworkAttachmentUrl(url: string): boolean {
32+
try {
33+
return BF_ATTACHMENT_RE.test(new URL(url).pathname);
34+
} catch {
35+
return false;
36+
}
37+
}
38+
39+
/** Rewrite Bot Framework attachment URLs to append /views/original, preserving query strings. */
40+
function rewriteBotFrameworkUrl(url: string): string {
41+
if (!isBotFrameworkAttachmentUrl(url)) return url;
42+
const parsed = new URL(url);
43+
parsed.pathname = parsed.pathname.replace(/\/+$/, "") + "/views/original";
44+
return parsed.toString();
45+
}
46+
2947
function resolveDownloadCandidate(att: MSTeamsAttachmentLike): DownloadCandidate | null {
3048
const contentType = normalizeContentType(att.contentType);
3149
const name = typeof att.name === "string" ? att.name.trim() : "";
@@ -62,8 +80,13 @@ function resolveDownloadCandidate(att: MSTeamsAttachmentLike): DownloadCandidate
6280
return null;
6381
}
6482

83+
// Bot Framework attachment URLs (…/v3/attachments/{id}) return JSON metadata;
84+
// append /views/original to retrieve the actual binary content.
85+
// Preserve any query string (e.g. signed URLs with ?sig=...).
86+
let url = rewriteBotFrameworkUrl(contentUrl);
87+
6588
return {
66-
url: contentUrl,
89+
url,
6790
fileHint: name || undefined,
6891
contentTypeHint: contentType,
6992
placeholder: inferPlaceholder({ contentType, fileName: name }),
@@ -194,8 +217,10 @@ export async function downloadMSTeamsAttachments(params: {
194217
continue;
195218
}
196219
seenUrls.add(inline.url);
220+
// Apply Bot Framework URL rewrite to inline <img src> URLs too.
221+
const rewrittenUrl = rewriteBotFrameworkUrl(inline.url);
197222
candidates.push({
198-
url: inline.url,
223+
url: rewrittenUrl,
199224
fileHint: inline.fileHint,
200225
contentTypeHint: inline.contentType,
201226
placeholder: inline.placeholder,

‎extensions/msteams/src/attachments/graph.ts‎

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ function normalizeGraphAttachment(att: GraphAttachment): MSTeamsAttachmentLike {
173173
* Download all hosted content from a Teams message (images, documents, etc.).
174174
* Renamed from downloadGraphHostedImages to support all file types.
175175
*/
176-
async function downloadGraphHostedContent(params: {
176+
export async function downloadGraphHostedContent(params: {
177177
accessToken: string;
178178
messageUrl: string;
179179
maxBytes: number;
@@ -191,24 +191,83 @@ async function downloadGraphHostedContent(params: {
191191
return { media: [], status: hosted.status, count: 0 };
192192
}
193193

194+
const fetchFn = params.fetchFn ?? fetch;
194195
const out: MSTeamsInboundMedia[] = [];
195196
for (const item of hosted.items) {
197+
let buffer: Buffer | undefined;
198+
let headerMime: string | undefined;
199+
200+
// Fast path: use contentBytes when the collection response includes them.
196201
const contentBytes = typeof item.contentBytes === "string" ? item.contentBytes : "";
197-
if (!contentBytes) {
198-
continue;
202+
if (contentBytes) {
203+
try {
204+
buffer = Buffer.from(contentBytes, "base64");
205+
} catch {
206+
// Fall through to $value fetch.
207+
}
199208
}
200-
let buffer: Buffer;
201-
try {
202-
buffer = Buffer.from(contentBytes, "base64");
203-
} catch {
209+
210+
// Graph collection responses typically return contentBytes as null.
211+
// Fetch the raw binary from the individual $value endpoint instead.
212+
if (!buffer && item.id) {
213+
try {
214+
const valueUrl = `${params.messageUrl}/hostedContents/${encodeURIComponent(item.id)}/$value`;
215+
const { response: valRes, release } = await fetchWithSsrFGuard({
216+
url: valueUrl,
217+
fetchImpl: fetchFn,
218+
init: { headers: { Authorization: `Bearer ${params.accessToken}` } },
219+
policy: params.ssrfPolicy,
220+
});
221+
try {
222+
if (valRes.ok) {
223+
const contentLength = Number(valRes.headers.get("content-length") ?? "0");
224+
if (contentLength > params.maxBytes) {
225+
// Skip oversized content without buffering.
226+
} else if (valRes.body) {
227+
// Stream with bounded read to avoid buffering oversized payloads
228+
// when content-length is missing or misreported.
229+
const chunks: Uint8Array[] = [];
230+
let totalBytes = 0;
231+
let oversized = false;
232+
const reader = valRes.body.getReader();
233+
try {
234+
for (;;) {
235+
const { done, value } = await reader.read();
236+
if (done) break;
237+
totalBytes += value.byteLength;
238+
if (totalBytes > params.maxBytes) {
239+
oversized = true;
240+
await reader.cancel();
241+
break;
242+
}
243+
chunks.push(value);
244+
}
245+
} finally {
246+
reader.releaseLock();
247+
}
248+
if (!oversized && chunks.length > 0) {
249+
buffer = Buffer.concat(chunks);
250+
headerMime = valRes.headers.get("content-type") ?? undefined;
251+
}
252+
}
253+
}
254+
} finally {
255+
await release();
256+
}
257+
} catch {
258+
// Network/fetch failure — skip this item.
259+
}
260+
}
261+
262+
if (!buffer) {
204263
continue;
205264
}
206265
if (buffer.byteLength > params.maxBytes) {
207266
continue;
208267
}
209268
const mime = await getMSTeamsRuntime().media.detectMime({
210269
buffer,
211-
headerMime: item.contentType ?? undefined,
270+
headerMime: headerMime ?? item.contentType ?? undefined,
212271
});
213272
// Download any file type, not just images
214273
try {

‎extensions/msteams/src/attachments/shared.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const DEFAULT_MEDIA_HOST_ALLOWLIST = [
4949
"asm.skype.com",
5050
"ams.skype.com",
5151
"media.ams.skype.com",
52-
// Bot Framework attachment URLs
52+
// Bot Framework attachment URLs — regional variants (smba, smba.eu, smba.ap, etc.)
5353
"trafficmanager.net",
5454
"blob.core.windows.net",
5555
"azureedge.net",
@@ -59,6 +59,10 @@ export const DEFAULT_MEDIA_HOST_ALLOWLIST = [
5959
export const DEFAULT_MEDIA_AUTH_HOST_ALLOWLIST = [
6060
"api.botframework.com",
6161
"botframework.com",
62+
// Bot Framework attachment service URLs — regional variants
63+
"smba.trafficmanager.net",
64+
"smba.eu.trafficmanager.net",
65+
"smba.ap.trafficmanager.net",
6266
"graph.microsoft.com",
6367
"graph.microsoft.us",
6468
"graph.microsoft.de",

0 commit comments

Comments
 (0)