Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upload return virtual host style url for bucket with dot in the name #6748

Open
3 of 4 tasks
changgengli opened this issue Dec 18, 2024 · 1 comment
Open
3 of 4 tasks
Assignees
Labels
bug This issue is a bug. p3 This is a minor priority issue response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days.

Comments

@changgengli
Copy link

changgengli commented Dec 18, 2024

Checkboxes for prior research

Describe the bug

Related issue:
#5490

Related Code
https://github.com/aws/aws-sdk-js-v3/blob/main/lib/lib-storage/src/Upload.ts#L171-L181

Why I think this is a bug:

  • The library should be smart enough to use path style when bucket name includes dot. It can do so. This allows the same S3 client instance can be used to interact with multiple buckets, without forcing path style for all the buckets.
  • This is inconsistent with multipart upload, the location of multipart upload output will use path style when bucket name includes dot, even if forcePathStyle is not set.
  • This is inconsistent with the behavior of V2.

Regression Issue

  • Select this option if this issue appears to be a regression.

SDK version number

@aws-sdk/[email protected]

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.17.0

Reproduction Steps

Use an s3 bucket with dots in its name

             
		const s3Client: S3Client = new S3({
			credentials,
			region: "us-west-2",
		})
		const upload = new Upload({
			params: {
				Bucket: "test.bucket.name",
				Key: "test/test.txt",
				Body: "aaaa",
				ContentType: "text/plain",
				ContentDisposition: "inline",
			},
			client: s3Client,
			partSize: 5 * 1024 * 1024,
		})
		const result = await upload.done()
		console.log(JSON.stringify(result, null, 2))

Observed Behavior

the location returned is not an accessible url
eg. https://example.bucket.s3.ap-south-1.amazonaws.com/key

Expected Behavior

it should use path style, eg. https://s3.ap-south-1.amazonaws.com/example.bucket/key

Possible Solution

No response

Additional Information/Context

No response

@changgengli changgengli added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 18, 2024
@aBurmeseDev aBurmeseDev self-assigned this Jan 7, 2025
@aBurmeseDev aBurmeseDev added investigating Issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Jan 8, 2025
@aBurmeseDev
Copy link
Member

Hi @changgengli - thanks for reaching out.

As you mentioned, in AWS SDK JavaScript v2, you can use s3ForcePathStyle whether to force path style URLs for S3 objects whereas in version 3, it was renamed to forcePathStyle.

Another notable change between versions is how Multipart Upload designed to work. In version 2, S3 client has ManagedUpload with upload() under the hood for uploading large objects (aka multipart upload). In v3, we introduced @aws-sdk/lib-storage package which supports all the features that was available in v2 upload(). In v3, user's expected to use forcePathStyle as s3ForcePathStyle was expected in v2. This's intended behavior and please use forcePathStyle in client initialization.

Here's documentation page on notable changes from v2 to v3 for reference: https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/migrating/notable-changes/

@aBurmeseDev aBurmeseDev added response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. p3 This is a minor priority issue and removed investigating Issue is being investigated and/or work is in progress to resolve the issue. labels Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p3 This is a minor priority issue response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants