@uppy/aws-s3: fix server generated keys not getting returned

[qxprakash]

fixes #6496

NO AI USED in code

we were always passing client generated keys from s3Opts methods ,

see :

in #onSuccess we're passing key returned from putObject

uppy/packages/@uppy/aws-s3/src/S3Uploader.ts

Lines 262 to 279 in 06ae391

	async #uploadNonMultipart(signal: AbortSignal): Promise<void> {
	const { location, key } = await this.#options.s3Client.putObject({
	key: this.#options.key,
	data: this.#data,
	fileType: this.#options.file.type || 'application/octet-stream',
	metadata: this.#options.metadata,
	onProgress: (bytesUploaded: number) => {
	this.#chunkState[0].uploaded = bytesUploaded
	this.#onProgress()
	},
	signal,
	})
	this.#onSuccess({
	location,
	key,
	})
	}

putObject directly returns the key it gets in params which is the client generated key

uppy/packages/@uppy/aws-s3/src/s3-client/S3mini.ts

Lines 179 to 201 in 06ae391

	public override async putObject({
	key,
	data,
	fileType = C.DEFAULT_STREAM_CONTENT_TYPE,
	onProgress,
	signal,
	}: IT.PutObjectParams) {
	this._checkKey(key)
	const { xhr, url } = await this.request({
	request: { method: 'PUT', key },
	data,
	onProgress,
	signal,
	contentType: fileType,
	})
	return {
	location: U.removeQueryString(url),
	etag: U.sanitizeETag(xhr.getResponseHeader('etag')),
	key,
	}
	}

people can modify these keys on their signing backends, when a signing backend stores the object under a different key:

• Single-part: upload succeeds, but upload-success reports a key that doesn't exist in the bucket. This is the reported issue in In @uppy/aws-s3 6.0, the key returned in file.response.body.key is the client-generated key, not the server-generated one #6496

• Multipart: the reported key was correct (it's parsed from S3's CompleteMultipartUpload response), but the client key was used internally for every part/complete/abort request and persisted as s3Multipart.key for Golden Retriever. That only works if the server maps client→server key deterministically on every call; a server that generates a unique key fails at the first uploadPart with NoSuchUpload.

Fix

• signRequest may now return { url, key }. request() resolves the key once (signedKey || request.key), and putObject / createMultipartUpload return it.
• key is optional: signers that return only { url } get exactly the previous behaviour, so this is non-breaking. Servers that sign for a key other than the one requested must return it the plugin has no way to detect the change otherwise.

[changeset-bot]

🦋 Changeset detected

Latest commit: ac1c708

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@uppy/aws-s3	Minor
uppy	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[qxprakash]

If the key your server signs for is not exactly the key it received, you must return it as key in the response. Otherwise Uppy assumes the object was stored under the key it requested.

This is the directive which we need add in our docs, sadly this wasn't the case previously, before the rewrite we used to return the key from the server

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[mifi]

Suggested change

	return { xhr, url, signedKey: signedKey || request.key }
	return { xhr, url, signedKey: signedKey }

i don't like that it means two things, does it need to?

[qxprakash]

yes it needs to, because what if the signer doesn't returns the key it has signed in it's response ? then we'll use the client generated key request.key is the client generated key, you can check the code paths #uploadNonMultipart -> putObject -> this.request the client generated key is propagated throughout, so we're using that same key in case the signing server doesn't sends it. if my explaination had muddled your understanding then let me know I'll try to elaborate a bit more 😂

[mifi]

i did an ai review: #6507

looks ok? then we can merge both