From 2cdeb88b4da3e865ddad78e3ddd271445b0d8157 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 10:36:23 +0530 Subject: [PATCH 01/13] Outline --- .../gallery/services/upload/remote.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index 976a7ca3b8..2bcb95553c 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -267,6 +267,63 @@ export class PhotosUploadHTTPClient { } } +/** + * Complete a multipart upload by reporting information about all the uploaded + * parts to the provided {@link completionURL}. + * + * @param completionURL The URL to which the final status of the uploaded parts + * should be reported to. + * + * @param reqBody Information about the parts which were uploaded. + * + * [Note: Multipart uploads] + * + * Multipart uploads are a mechanism to upload large files onto an remote + * storage bucket by breaking it into smaller chunks / "parts", uploading each + * part separately, and then reporting the consolidated information of all the + * uploaded parts to a URL that marks the upload as complete on remote. + * + * This allows greater resilience since uploads of individual parts can be + * retried independently without failing the entire upload on transient network + * issues. This also helps self hosters, since often cloud providers have limits + * to the size of single requests that they'll allow through (e.g. the + * Cloudflare free plan currently has a 100 MB request size limit). + * + * The flow is implemented in two ways: + * + * a. The normal way, where each requests is made to a remote S3 bucket directly + * using the presigned URL. + * + * b. Using workers, where the requests are proxied via a worker near to the + * user's network to speed the requests up. + * + * See the documentation of {@link shouldDisableCFUploadProxy} for more details + * about the via-worker flow. + * + * In both cases, the overall flow is roughly like the following: + * + * 1. Obtain multiple presigned URLs from remote (museum). The specific API call + * will be different (because of the different authentication mechanisms) + * when we're running in the context of the photos app and when we're running + * in the context of the public albums app. + * + * 2. Break the file to be uploaded into parts, and upload each part using a PUT + * request to one of the presigned URLs we got in step 1. There are two + * variants of this - one where we directly upload to the remote (S3), and + * one where we go via a worker. + * + * 3. Once all the parts have been uploaded, send a consolidated report of all + * the uploaded parts (the step 2's) to remote via another presigned + * "completion URL" that we also got in step 1. Like step 2, there are 2 + * variants of this - one where we directly tell the remote (S3), and one + * where we report via a worker. + * + */ +export const _completeMultipartUpload = async (completionURL: string) => { + +} + + /** * Lowest layer for file upload related HTTP operations when we're running in * the context of the public albums app. From e022e7ae5bb25c56d50f7cf9b65499dd74fc5ef5 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 10:46:02 +0530 Subject: [PATCH 02/13] 1 --- .../gallery/services/upload/remote.ts | 24 ++++++++++++------- .../gallery/services/upload/upload-service.ts | 7 +++++- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index 2bcb95553c..3a2c652b12 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -7,6 +7,7 @@ import { authenticatedRequestHeaders, ensureOk, retryAsyncOperation, + retryEnsuringHTTPOk, type PublicAlbumsCredentials, } from "ente-base/http"; import log from "ente-base/log"; @@ -271,10 +272,11 @@ export class PhotosUploadHTTPClient { * Complete a multipart upload by reporting information about all the uploaded * parts to the provided {@link completionURL}. * - * @param completionURL The URL to which the final status of the uploaded parts - * should be reported to. + * @param completionURL A presigned URL to which the final status of the + * uploaded parts should be reported to. * - * @param reqBody Information about the parts which were uploaded. + * @param reqBody A XML string containing information about the parts which were + * uploaded. * * [Note: Multipart uploads] * @@ -317,12 +319,18 @@ export class PhotosUploadHTTPClient { * "completion URL" that we also got in step 1. Like step 2, there are 2 * variants of this - one where we directly tell the remote (S3), and one * where we report via a worker. - * */ -export const _completeMultipartUpload = async (completionURL: string) => { - -} - +export const _completeMultipartUpload = async ( + completionURL: string, + reqBody: string, +) => + retryEnsuringHTTPOk(() => + fetch(completionURL, { + method: "POST", + headers: { "Content-Type": "text/xml" }, + body: reqBody, + }), + ); /** * Lowest layer for file upload related HTTP operations when we're running in diff --git a/web/packages/gallery/services/upload/upload-service.ts b/web/packages/gallery/services/upload/upload-service.ts index 2ce0f46852..64300f099d 100644 --- a/web/packages/gallery/services/upload/upload-service.ts +++ b/web/packages/gallery/services/upload/upload-service.ts @@ -53,6 +53,7 @@ import { } from "."; import { tryParseEpochMicrosecondsFromFileName } from "./date"; import { + _completeMultipartUpload, PhotosUploadHTTPClient, PublicAlbumsUploadHTTPClient, type ObjectUploadURL, @@ -1605,7 +1606,11 @@ async function uploadStreamUsingMultipart( if (!isCFUploadProxyDisabled) { await photosHTTPClient.completeMultipartUploadV2(completeURL, cBody); } else { - await photosHTTPClient.completeMultipartUpload(completeURL, cBody); + if (process.env.NEXT_PUBLIC_ENTE_WIP_MP) { + await _completeMultipartUpload(completeURL, cBody); + } else { + await photosHTTPClient.completeMultipartUpload(completeURL, cBody); + } } return { objectKey: multipartUploadURLs.objectKey, fileSize }; From 073d2c56842792a29d759446bdcda29d3897753b Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 11:06:39 +0530 Subject: [PATCH 03/13] 2 --- web/packages/base/http.ts | 3 ++- web/packages/gallery/services/upload/remote.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/web/packages/base/http.ts b/web/packages/base/http.ts index 6a12185c56..6b43bb4e9a 100644 --- a/web/packages/base/http.ts +++ b/web/packages/base/http.ts @@ -18,7 +18,8 @@ export const authenticatedRequestHeaders = async () => ({ /** * Return headers that should be passed alongwith (almost) all unauthenticated - * `fetch` calls that we make to our API servers. + * `fetch` calls that we make to our remotes like our API servers (museum), or + * to presigned URLs that are handled by the S3 storage buckets themselves. * * - The client package name. */ diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index 3a2c652b12..b87291aec7 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -6,6 +6,7 @@ import { authenticatedPublicAlbumsRequestHeaders, authenticatedRequestHeaders, ensureOk, + publicRequestHeaders, retryAsyncOperation, retryEnsuringHTTPOk, type PublicAlbumsCredentials, @@ -327,7 +328,7 @@ export const _completeMultipartUpload = async ( retryEnsuringHTTPOk(() => fetch(completionURL, { method: "POST", - headers: { "Content-Type": "text/xml" }, + headers: { ...publicRequestHeaders(), "Content-Type": "text/xml" }, body: reqBody, }), ); From 4772557f7ab98fd897ebd75f207768058a234fdc Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 11:32:31 +0530 Subject: [PATCH 04/13] Tweak --- .../gallery/services/upload/upload-service.ts | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/web/packages/gallery/services/upload/upload-service.ts b/web/packages/gallery/services/upload/upload-service.ts index 64300f099d..6a788185bd 100644 --- a/web/packages/gallery/services/upload/upload-service.ts +++ b/web/packages/gallery/services/upload/upload-service.ts @@ -1572,8 +1572,8 @@ async function uploadStreamUsingMultipart( ] of multipartUploadURLs.partURLs.entries()) { abortIfCancelled(); - const uploadChunk = await combineChunksToFormUploadPart(streamReader); - fileSize += uploadChunk.length; + const uploadPart = await nextMultipartUploadPart(streamReader); + fileSize += uploadPart.length; const progressTracker = makeProgressTracker( fileLocalID, percentPerPart, @@ -1583,13 +1583,13 @@ async function uploadStreamUsingMultipart( if (!isCFUploadProxyDisabled) { eTag = await photosHTTPClient.putFilePartV2( fileUploadURL, - uploadChunk, + uploadPart, progressTracker, ); } else { eTag = await photosHTTPClient.putFilePart( fileUploadURL, - uploadChunk, + uploadPart, progressTracker, ); } @@ -1616,16 +1616,19 @@ async function uploadStreamUsingMultipart( return { objectKey: multipartUploadURLs.objectKey, fileSize }; } -async function combineChunksToFormUploadPart( +/** + * Construct byte arrays, up to 20 MB each, containing the contents of (up to) + * the next 5 {@link streamEncryptionChunkSize} chunks read from the given + * {@link streamReader}. + */ +const nextMultipartUploadPart = async ( streamReader: ReadableStreamDefaultReader, -) { - const combinedChunks = []; +) => { + const chunks = []; for (let i = 0; i < multipartChunksPerPart; i++) { const { done, value: chunk } = await streamReader.read(); - if (done) { - break; - } - combinedChunks.push(chunk); + if (done) break; + chunks.push(chunk); } - return mergeUint8Arrays(combinedChunks); -} + return mergeUint8Arrays(chunks); +}; From 89294f2a768640867ba7cd2e8a715980658a17e4 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 11:52:05 +0530 Subject: [PATCH 05/13] Outline --- .../gallery/services/upload/remote.ts | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index b87291aec7..53b70712f4 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -269,6 +269,43 @@ export class PhotosUploadHTTPClient { } } +/** + * Information about an individual part of a multipart upload. + * + * See: [Note: Multipart uploads]. + */ +interface MultipartUploadPartInfo { + /** + * The part number (1-indexed). + * + * The part number indicates the sequential ordering (starting from 1) where + * this part belongs in the overall file's data. + */ + partNumber: number; + /** + * The part "ETag". + * + * This is the value of the "ETag" header in the remote response we received + * when the part was uploaded. + */ + etag: string; +} + +/** + * Construct an XML string of the format expected as the request body for + * {@link _completeMultipartUpload} or {@link _completeMultipartUploadViaProxy}. + * + * @param parts Information about the parts that were uploaded. + */ +export const createMultipartUploadRequestBody = ( + parts: MultipartUploadPartInfo[], +): string => { + // To avoid introducing a dependency on a XML library, we construct the + // requisite XML by hand. + const result = ""; + return result; +}; + /** * Complete a multipart upload by reporting information about all the uploaded * parts to the provided {@link completionURL}. From 201ef60f07303f5e15b7579a795657fcfce9f93a Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 12:32:50 +0530 Subject: [PATCH 06/13] Outline --- .../gallery/services/upload/remote.ts | 61 ++++++++++++++++--- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index 53b70712f4..beb031bffd 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -270,23 +270,27 @@ export class PhotosUploadHTTPClient { } /** - * Information about an individual part of a multipart upload. + * Information about an individual part of a multipart upload that has been + * uploaded to the remote (S3 or proxy). * * See: [Note: Multipart uploads]. */ -interface MultipartUploadPartInfo { +interface MultipartCompletedPart { /** * The part number (1-indexed). * - * The part number indicates the sequential ordering (starting from 1) where - * this part belongs in the overall file's data. + * The part number indicates the sequential ordering where this part belongs + * in the overall file's data. + * + * The part number must start at 1 and the part numbers that get passed to + * {@link createMultipartUploadRequestBody} must be consecutive. */ partNumber: number; /** * The part "ETag". * - * This is the value of the "ETag" header in the remote response we received - * when the part was uploaded. + * This is the Entity tag (retrieved as the "ETag" response header) returned + * by remote when the part was uploaded. */ etag: string; } @@ -298,10 +302,53 @@ interface MultipartUploadPartInfo { * @param parts Information about the parts that were uploaded. */ export const createMultipartUploadRequestBody = ( - parts: MultipartUploadPartInfo[], + parts: MultipartCompletedPart[], ): string => { // To avoid introducing a dependency on a XML library, we construct the // requisite XML by hand. + // + // Example: + // + // + // + // 1 + // "1b3e6cdb1270c0b664076f109a7137c1" + // + // + // 2 + // "6049d6384a9e65694c833a3aca6584fd" + // + // + // 3 + // "331747eae8068f03b844e6f28cc0ed23" + // + // + // + // + // Spec: + // https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html + // + // + // + // integer + // string + // + // ... + // + // + // Note that in the example given on the spec page, the etag strings are quoted: + // + // + // + // 1 + // "a54357aff0632cce46d942af68356b38" + // + // ... + // + // + // No extra quotes need to be added, the etag values we get from remote + // already quoted, we just need to pass them verbatim. + const result = ""; return result; }; From 24c66a9b6b98e1a9857a8840ad9d424d2fd0a063 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 12:37:59 +0530 Subject: [PATCH 07/13] impl --- web/packages/gallery/services/upload/remote.ts | 11 +++++++---- .../gallery/services/upload/upload-service.ts | 13 ++++++++++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index beb031bffd..24ed0e3a1c 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -275,7 +275,7 @@ export class PhotosUploadHTTPClient { * * See: [Note: Multipart uploads]. */ -interface MultipartCompletedPart { +export interface MultipartCompletedPart { /** * The part number (1-indexed). * @@ -292,7 +292,7 @@ interface MultipartCompletedPart { * This is the Entity tag (retrieved as the "ETag" response header) returned * by remote when the part was uploaded. */ - etag: string; + eTag: string; } /** @@ -349,8 +349,11 @@ export const createMultipartUploadRequestBody = ( // No extra quotes need to be added, the etag values we get from remote // already quoted, we just need to pass them verbatim. - const result = ""; - return result; + const resultParts = parts.map( + (part) => + `${part.partNumber}${part.eTag}`, + ); + return `${resultParts.join("\n")}`; }; /** diff --git a/web/packages/gallery/services/upload/upload-service.ts b/web/packages/gallery/services/upload/upload-service.ts index 6a788185bd..93ebf91ed3 100644 --- a/web/packages/gallery/services/upload/upload-service.ts +++ b/web/packages/gallery/services/upload/upload-service.ts @@ -54,8 +54,10 @@ import { import { tryParseEpochMicrosecondsFromFileName } from "./date"; import { _completeMultipartUpload, + createMultipartUploadRequestBody, PhotosUploadHTTPClient, PublicAlbumsUploadHTTPClient, + type MultipartCompletedPart, type ObjectUploadURL, } from "./remote"; import type { ParsedMetadataJSON } from "./takeout"; @@ -1565,6 +1567,7 @@ async function uploadStreamUsingMultipart( const percentPerPart = RANDOM_PERCENTAGE_PROGRESS_FOR_PUT() / uploadPartCount; const partEtags: PartEtag[] = []; + const completedParts: MultipartCompletedPart[] = []; let fileSize = 0; for (const [ index, @@ -1594,6 +1597,7 @@ async function uploadStreamUsingMultipart( ); } partEtags.push({ PartNumber: index + 1, ETag: eTag }); + completedParts.push({ partNumber: index + 1, eTag }); } const { done } = await streamReader.read(); if (!done) throw new Error("More chunks than expected"); @@ -1603,11 +1607,18 @@ async function uploadStreamUsingMultipart( { CompleteMultipartUpload: { Part: partEtags } }, { compact: true, ignoreComment: true, spaces: 4 }, ); + if (process.env.NEXT_PUBLIC_ENTE_WIP_MP) { + console.log(cBody); + console.log(createMultipartUploadRequestBody(completedParts)); + } if (!isCFUploadProxyDisabled) { await photosHTTPClient.completeMultipartUploadV2(completeURL, cBody); } else { if (process.env.NEXT_PUBLIC_ENTE_WIP_MP) { - await _completeMultipartUpload(completeURL, cBody); + await _completeMultipartUpload( + completeURL, + createMultipartUploadRequestBody(completedParts), + ); } else { await photosHTTPClient.completeMultipartUpload(completeURL, cBody); } From b372ba47ba064782001d31f793d0cdcb94306320 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 13:20:06 +0530 Subject: [PATCH 08/13] Swap --- web/packages/gallery/services/upload/remote.ts | 16 +--------------- .../gallery/services/upload/upload-service.ts | 14 +++++--------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index 24ed0e3a1c..06ce4eed07 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -236,20 +236,6 @@ export class PhotosUploadHTTPClient { } } - async completeMultipartUpload(completeURL: string, reqBody: unknown) { - try { - await retryAsyncOperation(() => - // @ts-ignore - HTTPService.post(completeURL, reqBody, null, { - "content-type": "text/xml", - }), - ); - } catch (e) { - log.error("put file in parts failed", e); - throw e; - } - } - async completeMultipartUploadV2(completeURL: string, reqBody: unknown) { try { const origin = await uploaderOrigin(); @@ -408,7 +394,7 @@ export const createMultipartUploadRequestBody = ( * variants of this - one where we directly tell the remote (S3), and one * where we report via a worker. */ -export const _completeMultipartUpload = async ( +export const completeMultipartUpload = async ( completionURL: string, reqBody: string, ) => diff --git a/web/packages/gallery/services/upload/upload-service.ts b/web/packages/gallery/services/upload/upload-service.ts index 93ebf91ed3..dd93016902 100644 --- a/web/packages/gallery/services/upload/upload-service.ts +++ b/web/packages/gallery/services/upload/upload-service.ts @@ -53,7 +53,7 @@ import { } from "."; import { tryParseEpochMicrosecondsFromFileName } from "./date"; import { - _completeMultipartUpload, + completeMultipartUpload, createMultipartUploadRequestBody, PhotosUploadHTTPClient, PublicAlbumsUploadHTTPClient, @@ -1614,14 +1614,10 @@ async function uploadStreamUsingMultipart( if (!isCFUploadProxyDisabled) { await photosHTTPClient.completeMultipartUploadV2(completeURL, cBody); } else { - if (process.env.NEXT_PUBLIC_ENTE_WIP_MP) { - await _completeMultipartUpload( - completeURL, - createMultipartUploadRequestBody(completedParts), - ); - } else { - await photosHTTPClient.completeMultipartUpload(completeURL, cBody); - } + await completeMultipartUpload( + completeURL, + createMultipartUploadRequestBody(completedParts), + ); } return { objectKey: multipartUploadURLs.objectKey, fileSize }; From 67140fe7f20a2869ebbdb515cc58f95e0efc4e55 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 13:52:33 +0530 Subject: [PATCH 09/13] s2 --- .../gallery/services/upload/remote.ts | 22 ++++++++++++++----- .../gallery/services/upload/upload-service.ts | 9 +------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index 06ce4eed07..cec52e7b31 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -267,9 +267,6 @@ export interface MultipartCompletedPart { * * The part number indicates the sequential ordering where this part belongs * in the overall file's data. - * - * The part number must start at 1 and the part numbers that get passed to - * {@link createMultipartUploadRequestBody} must be consecutive. */ partNumber: number; /** @@ -349,8 +346,8 @@ export const createMultipartUploadRequestBody = ( * @param completionURL A presigned URL to which the final status of the * uploaded parts should be reported to. * - * @param reqBody A XML string containing information about the parts which were - * uploaded. + * @param completedParts Information about all the parts of the file that have + * been uploaded. The part numbers must start at 1 and must be consecutive. * * [Note: Multipart uploads] * @@ -395,6 +392,21 @@ export const createMultipartUploadRequestBody = ( * where we report via a worker. */ export const completeMultipartUpload = async ( + completionURL: string, + completedParts: MultipartCompletedPart[], +) => + retryEnsuringHTTPOk(() => + fetch(completionURL, { + method: "POST", + headers: { ...publicRequestHeaders(), "Content-Type": "text/xml" }, + body: createMultipartUploadRequestBody(completedParts), + }), + ); + +/** + * Variant of {@link completeMultipartUpload} that uses the CF worker. + */ +export const completeMultipartUploadViaProxy = async ( completionURL: string, reqBody: string, ) => diff --git a/web/packages/gallery/services/upload/upload-service.ts b/web/packages/gallery/services/upload/upload-service.ts index dd93016902..eaf89f70ac 100644 --- a/web/packages/gallery/services/upload/upload-service.ts +++ b/web/packages/gallery/services/upload/upload-service.ts @@ -1607,17 +1607,10 @@ async function uploadStreamUsingMultipart( { CompleteMultipartUpload: { Part: partEtags } }, { compact: true, ignoreComment: true, spaces: 4 }, ); - if (process.env.NEXT_PUBLIC_ENTE_WIP_MP) { - console.log(cBody); - console.log(createMultipartUploadRequestBody(completedParts)); - } if (!isCFUploadProxyDisabled) { await photosHTTPClient.completeMultipartUploadV2(completeURL, cBody); } else { - await completeMultipartUpload( - completeURL, - createMultipartUploadRequestBody(completedParts), - ); + await completeMultipartUpload(completeURL, completedParts); } return { objectKey: multipartUploadURLs.objectKey, fileSize }; From ad87470c251e205a2b2ebdb3a864fd022fcdd428 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 13:56:40 +0530 Subject: [PATCH 10/13] 2 --- .../gallery/services/upload/remote.ts | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index cec52e7b31..9f84e1185d 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -391,7 +391,7 @@ export const createMultipartUploadRequestBody = ( * variants of this - one where we directly tell the remote (S3), and one * where we report via a worker. */ -export const completeMultipartUpload = async ( +export const completeMultipartUpload = ( completionURL: string, completedParts: MultipartCompletedPart[], ) => @@ -408,15 +408,21 @@ export const completeMultipartUpload = async ( */ export const completeMultipartUploadViaProxy = async ( completionURL: string, - reqBody: string, -) => - retryEnsuringHTTPOk(() => - fetch(completionURL, { + completedParts: MultipartCompletedPart[], +) => { + const origin = await uploaderOrigin(); + return retryEnsuringHTTPOk(() => + fetch(`${origin}/multipart-complete`, { method: "POST", - headers: { ...publicRequestHeaders(), "Content-Type": "text/xml" }, - body: reqBody, + headers: { + ...publicRequestHeaders(), + "Content-Type": "text/xml", + "UPLOAD-URL": completionURL, + }, + body: createMultipartUploadRequestBody(completedParts), }), ); +}; /** * Lowest layer for file upload related HTTP operations when we're running in From 5b38ef394bcb6e776e8da9493967dde3936a35dd Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 14:02:02 +0530 Subject: [PATCH 11/13] More debug info --- web/apps/photos/src/components/FileList.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/web/apps/photos/src/components/FileList.tsx b/web/apps/photos/src/components/FileList.tsx index 1452befb9c..f1ae399605 100644 --- a/web/apps/photos/src/components/FileList.tsx +++ b/web/apps/photos/src/components/FileList.tsx @@ -311,7 +311,15 @@ export const FileList: React.FC = ({ return timeStampList; } // TODO(RE): Remove after audit. - if (isDevBuild) throw new Error("Unexpected footer change"); + if ( + isDevBuild && + (footer || + publicCollectionGalleryContext.credentials || + showAppDownloadBanner) + ) { + console.log({ timeStampList, footer, showAppDownloadBanner }); + throw new Error("Unexpected footer change"); + } if (footer) { return [ ...timeStampList, From d19a0fccda13ac748c36e858001c02eb376ef15f Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 16:17:09 +0530 Subject: [PATCH 12/13] Remove dependency --- web/apps/photos/package.json | 3 +- .../gallery/services/upload/remote.ts | 42 +++++++------------ .../gallery/services/upload/upload-service.ts | 20 ++------- web/yarn.lock | 12 ------ 4 files changed, 21 insertions(+), 56 deletions(-) diff --git a/web/apps/photos/package.json b/web/apps/photos/package.json index ac79884f49..4de13c539e 100644 --- a/web/apps/photos/package.json +++ b/web/apps/photos/package.json @@ -22,8 +22,7 @@ "react-virtualized-auto-sizer": "^1.0.26", "react-window": "^1.8.11", "sanitize-filename": "^1.6.3", - "similarity-transformation": "^0.0.1", - "xml-js": "^1.6.11" + "similarity-transformation": "^0.0.1" }, "devDependencies": { "@types/node": "^22.15.18", diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index 9f84e1185d..9cbb30a3c6 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -20,12 +20,18 @@ import { z } from "zod"; import type { MultipartUploadURLs, UploadFile } from "./upload-service"; /** - * A pre-signed URL alongwith the associated object key. + * A pre-signed URL alongwith the associated object key that is later used to + * refer to file contents (the "object") that were uploaded to this URL. */ const ObjectUploadURL = z.object({ - /** A pre-signed URL that can be used to upload data to S3. */ + /** + * A pre-signed URL that can be used to upload data to an S3-compatible + * remote. + */ objectKey: z.string(), - /** The objectKey with which remote will refer to this object. */ + /** + * The objectKey with which remote (museum) will refer to this object. + */ url: z.string(), }); @@ -235,24 +241,6 @@ export class PhotosUploadHTTPClient { throw e; } } - - async completeMultipartUploadV2(completeURL: string, reqBody: unknown) { - try { - const origin = await uploaderOrigin(); - await retryAsyncOperation(() => - HTTPService.post( - `${origin}/multipart-complete`, - reqBody, - // @ts-ignore - null, - { "content-type": "text/xml", "UPLOAD-URL": completeURL }, - ), - ); - } catch (e) { - log.error("put file in parts failed", e); - throw e; - } - } } /** @@ -280,11 +268,12 @@ export interface MultipartCompletedPart { /** * Construct an XML string of the format expected as the request body for - * {@link _completeMultipartUpload} or {@link _completeMultipartUploadViaProxy}. + * {@link _completeMultipartUpload} or + * {@link _completeMultipartUploadViaWorker}. * * @param parts Information about the parts that were uploaded. */ -export const createMultipartUploadRequestBody = ( +const createMultipartUploadRequestBody = ( parts: MultipartCompletedPart[], ): string => { // To avoid introducing a dependency on a XML library, we construct the @@ -388,8 +377,9 @@ export const createMultipartUploadRequestBody = ( * 3. Once all the parts have been uploaded, send a consolidated report of all * the uploaded parts (the step 2's) to remote via another presigned * "completion URL" that we also got in step 1. Like step 2, there are 2 - * variants of this - one where we directly tell the remote (S3), and one - * where we report via a worker. + * variants of this - one where we directly tell the remote (S3) + * ({@link completeMultipartUpload}), and one where we report via a worker + * ({@link completeMultipartUploadViaWorker}). */ export const completeMultipartUpload = ( completionURL: string, @@ -406,7 +396,7 @@ export const completeMultipartUpload = ( /** * Variant of {@link completeMultipartUpload} that uses the CF worker. */ -export const completeMultipartUploadViaProxy = async ( +export const completeMultipartUploadViaWorker = async ( completionURL: string, completedParts: MultipartCompletedPart[], ) => { diff --git a/web/packages/gallery/services/upload/upload-service.ts b/web/packages/gallery/services/upload/upload-service.ts index eaf89f70ac..fb7beff2a3 100644 --- a/web/packages/gallery/services/upload/upload-service.ts +++ b/web/packages/gallery/services/upload/upload-service.ts @@ -44,7 +44,6 @@ import { settingsSnapshot } from "ente-new/photos/services/settings"; import { CustomError, handleUploadError } from "ente-shared/error"; import { mergeUint8Arrays } from "ente-utils/array"; import { ensureInteger, ensureNumber } from "ente-utils/ensure"; -import * as convert from "xml-js"; import type { UploadableUploadItem, UploadItem } from "."; import { RANDOM_PERCENTAGE_PROGRESS_FOR_PUT, @@ -54,7 +53,7 @@ import { import { tryParseEpochMicrosecondsFromFileName } from "./date"; import { completeMultipartUpload, - createMultipartUploadRequestBody, + completeMultipartUploadViaWorker, PhotosUploadHTTPClient, PublicAlbumsUploadHTTPClient, type MultipartCompletedPart, @@ -1543,11 +1542,6 @@ const uploadToBucket = async ( } }; -interface PartEtag { - PartNumber: number; - ETag: string; -} - async function uploadStreamUsingMultipart( fileLocalID: number, dataStream: EncryptedFileStream, @@ -1566,7 +1560,6 @@ async function uploadStreamUsingMultipart( const streamReader = stream.getReader(); const percentPerPart = RANDOM_PERCENTAGE_PROGRESS_FOR_PUT() / uploadPartCount; - const partEtags: PartEtag[] = []; const completedParts: MultipartCompletedPart[] = []; let fileSize = 0; for (const [ @@ -1596,21 +1589,16 @@ async function uploadStreamUsingMultipart( progressTracker, ); } - partEtags.push({ PartNumber: index + 1, ETag: eTag }); completedParts.push({ partNumber: index + 1, eTag }); } const { done } = await streamReader.read(); if (!done) throw new Error("More chunks than expected"); - const completeURL = multipartUploadURLs.completeURL; - const cBody = convert.js2xml( - { CompleteMultipartUpload: { Part: partEtags } }, - { compact: true, ignoreComment: true, spaces: 4 }, - ); + const completionURL = multipartUploadURLs.completeURL; if (!isCFUploadProxyDisabled) { - await photosHTTPClient.completeMultipartUploadV2(completeURL, cBody); + await completeMultipartUploadViaWorker(completionURL, completedParts); } else { - await completeMultipartUpload(completeURL, completedParts); + await completeMultipartUpload(completionURL, completedParts); } return { objectKey: multipartUploadURLs.objectKey, fileSize }; diff --git a/web/yarn.lock b/web/yarn.lock index 8685b8af75..158cb5b835 100644 --- a/web/yarn.lock +++ b/web/yarn.lock @@ -3643,11 +3643,6 @@ sanitize-filename@^1.6.3: dependencies: truncate-utf8-bytes "^1.0.0" -sax@^1.2.4: - version "1.4.1" - resolved "https://registry.yarnpkg.com/sax/-/sax-1.4.1.tgz#44cc8988377f126304d3b3fc1010c733b929ef0f" - integrity sha512-+aWOz7yVScEGoKNd4PA10LZ8sk0A/z5+nXQG5giUO5rprX9jgYsTdov9qCchZiPIZezbZH+jRut8nPodFAX4Jg== - scheduler@^0.26.0: version "0.26.0" resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.26.0.tgz#4ce8a8c2a2095f13ea11bf9a445be50c555d6337" @@ -4250,13 +4245,6 @@ wrap-ansi@^7.0.0: string-width "^4.1.0" strip-ansi "^6.0.0" -xml-js@^1.6.11: - version "1.6.11" - resolved "https://registry.yarnpkg.com/xml-js/-/xml-js-1.6.11.tgz#927d2f6947f7f1c19a316dd8eea3614e8b18f8e9" - integrity sha512-7rVi2KMfwfWFl+GpPg6m80IVMWXLRjO+PxTq7V2CDhoGak0wzYzFgUY2m4XJ47OGdXd8eLE8EmwfAmdjw7lC1g== - dependencies: - sax "^1.2.4" - y18n@^5.0.5: version "5.0.8" resolved "https://registry.yarnpkg.com/y18n/-/y18n-5.0.8.tgz#7f4934d0f7ca8c56f95314939ddcd2dd91ce1d55" From 7e7751b5beb3cb4f7885b6a4dcfc7b818d8e9a08 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 21 May 2025 16:27:25 +0530 Subject: [PATCH 13/13] Nicer --- web/packages/gallery/services/upload/remote.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/gallery/services/upload/remote.ts b/web/packages/gallery/services/upload/remote.ts index 9cbb30a3c6..46007067d9 100644 --- a/web/packages/gallery/services/upload/remote.ts +++ b/web/packages/gallery/services/upload/remote.ts @@ -325,7 +325,7 @@ const createMultipartUploadRequestBody = ( (part) => `${part.partNumber}${part.eTag}`, ); - return `${resultParts.join("\n")}`; + return `\n${resultParts.join("\n")}\n`; }; /**