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

Buckets with periods are ignored when using vhost-style addressing #2395

Closed
sa7mon opened this issue Nov 28, 2023 · 3 comments · Fixed by #2417
Closed

Buckets with periods are ignored when using vhost-style addressing #2395

sa7mon opened this issue Nov 28, 2023 · 3 comments · Fixed by #2417
Assignees
Labels
bug This issue is a bug. p1 This is a high priority issue

Comments

@sa7mon
Copy link

sa7mon commented Nov 28, 2023

Describe the bug

When:

  • an EndpointResolver is supplied
  • the endpoint has a non-wildcard SSL cert
  • the endpoint uses vhost-style addressing
  • a bucket name has a period in it

The HTTP request created is missing the bucket name entirely.

Expected Behavior

The SDK should treat a bucket name containing a period the same as one without one.

Current Behavior

The HTTP request created is missing the bucket name entirely

Reproduction Steps

Using the following main.go as an example, run it in an environment with an HTTP proxy configured to view requests and the http_proxy and https_proxy environment variables set. I will be using mitmproxy.

package main

import (
	"context"
	"crypto/tls"
	"errors"
	"github.com/aws/aws-sdk-go-v2/aws"
	awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/feature/s3/manager"
	"github.com/aws/aws-sdk-go-v2/service/s3"
	"github.com/aws/aws-sdk-go-v2/service/s3/types"
	log "github.com/sirupsen/logrus"
	"net/http"
)

func main() {
	cfg, err := config.LoadDefaultConfig(
		context.TODO(),
		config.WithEndpointResolverWithOptions(
			aws.EndpointResolverWithOptionsFunc(func(service, region string, options ...interface{}) (aws.Endpoint, error) {
				return aws.Endpoint{
					URL: "https://us-east-1.linodeobjects.com",
				}, nil
			})),
		config.WithCredentialsProvider(aws.AnonymousCredentials{}),
		config.WithHTTPClient(&http.Client{Transport: &http.Transport{
			Proxy:           http.ProxyFromEnvironment,
			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
		}}),
	)
	if err != nil {
		panic(err)
	}

	// use vhost style addressing
	addrStyleOption := func(o *s3.Options) { o.UsePathStyle = false }

	// create client
	s3client := s3.NewFromConfig(cfg, addrStyleOption)
	_, regionErr := manager.GetBucketRegion(context.TODO(), s3client, "mybucket")

	if regionErr == nil {
		log.Println("no error - bucket exists")
		return
	}

	var bnf manager.BucketNotFound
	var nsb *types.NoSuchBucket
	var re2 *awshttp.ResponseError
	if errors.As(regionErr, &bnf) {
		log.Println("BucketNotFound")
		return
	} else if errors.As(regionErr, &nsb) {
		log.Println("BucketNotFound")
		return
	} else if errors.As(regionErr, &re2) && re2.HTTPStatusCode() == 403 {
		log.Println("AccessDenied")
		return
	} else {
		log.WithError(regionErr).Println("unknown error")
		return
	}
}

After running the above example, observe the HTTP request looks like this:

HEAD https://mybucket.us-east-1.linodeobjects.com/ HTTP/1.1

Host: mybucket.us-east-1.linodeobjects.com
User-Agent: aws-sdk-go-v2/1.21.2 os/linux lang/go#1.19.12 md/GOOS#linux md/GOARCH#arm64 api/s3#1.40.2
Accept-Encoding: identity
Amz-Sdk-Invocation-Id: 15566266-e21f-41d9-b4ab-f2a9f242ef5d
Amz-Sdk-Request: attempt=1; max=3
X-Amz-Content-Sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

Now change the bucket name in the code snippet to my.bucket. Observe the request looks like this:

HEAD https://us-east-1.linodeobjects.com/ HTTP/1.1

Host: us-east-1.linodeobjects.com
User-Agent: aws-sdk-go-v2/1.21.2 os/linux lang/go#1.19.12 md/GOOS#linux md/GOARCH#arm64 api/s3#1.40.2
Accept-Encoding: identity
Amz-Sdk-Invocation-Id: c7561a79-ea76-41f5-ab5e-94be75d065d9
Amz-Sdk-Request: attempt=1; max=3
X-Amz-Content-Sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855

As you can see, the bucket name is missing from the second request.

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

v1.21.2

Compiler and Version used

go version go1.19.12 linux/arm64

Operating System and version

Docker - golang:1.19-alpine

@sa7mon sa7mon added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 28, 2023
@isaiahvita isaiahvita self-assigned this Nov 29, 2023
@RanVaknin RanVaknin added p1 This is a high priority issue queued This issues is on the AWS team's backlog and removed needs-triage This issue or PR still needs to be triaged. labels Nov 30, 2023
@lucix-aws lucix-aws assigned lucix-aws and unassigned isaiahvita Dec 5, 2023
@lucix-aws
Copy link
Contributor

Regression from EndpointResolverV2 refactor - this bucket should be appearing in the path. Will be addressed by #2417.

Note that the bucket name is and has always been considered "un-hoistable" by us due to the endpoint scheme being HTTPS and the bucket name having the .s. If your particular TLS setup allows you to circumvent this and you want the virtual-hosting style you will have to own that logic (that's consistent with our pre-v2 behavior).

@lucix-aws lucix-aws removed the queued This issues is on the AWS team's backlog label Dec 7, 2023
Copy link

github-actions bot commented Dec 7, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@sa7mon
Copy link
Author

sa7mon commented Dec 7, 2023

@lucix-aws Do you happen to have an example of that logic? I did a deep dive in the various config options for a client and couldn't find a working incantation.

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. p1 This is a high priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants