jrochkind / faster_s3_url

Optimized generation of public and presigned AWS S3 GET URLs in ruby faster
MIT License
84 stars 7 forks source link

When using the host option the url does not include the bucket name #11

Closed BenKanouse closed 20 hours ago

BenKanouse commented 1 week ago

If the host option is passed into the Builder class the returned url does not include the bucket name. So I do not think this option is working as intended.

I was able to alter this spec to assert that the bucket name was in the url and it fails.

My altered spec:

    describe "custom host" do
      let(:host) { "my.example.com" }

      it "is correct" do
        expect(builder.public_url(object_key)).to eq("https://#{host}/#{object_key}")
        expect(builder.public_url(object_key)).to include(bucket_name) # new assertion
      end
    end

failure message:

  1) FasterS3Url#public_url custom host is correct
     Failure/Error: expect(builder.public_url(object_key)).to include(bucket_name)
       expected "https://my.example.com/some/directory/file.jpg" to include "my-bucket"
     # ./spec/builder_spec.rb:70:in `block (4 levels) in <top (required)>'

It is hard to say what the correct behavior should be since amazon has an endpoint option and not a host option. My guess is that we would want https://my-bucket.my.example.com/some/directory/file.jpg which is what amazon returns with an endpoint of https://my.example.com.

jrochkind commented 20 hours ago

Hi, thanks for the clearly communicated issue, and for getting in touch!

OK, so host -- you are right that when I implemented/documented it I hadn't totally thought it ought and it didn't quite work in that way.

But I have ended up using it for when you have a CloudFront distribution (or any other CDN) on top of S3. For public unsigned urls, you just need to change the hostname to be your CDN hostname. And this is a convenient tool to do it, in a speedy way (much speedier than AWS SDK), applying appropriate (a bit idiosyncratic) S3 escaping rules for keys, etc.

So it is correct that it doesn't include the bucket name -- it is simply replacing the host name with your own custom specified hostname, that might be a CDN or other kind of proxy. That's all it is, and I have had a use for it myself, so would like to leave it in!

When I wrote it, I think I didn't totally understand the use cases (that I ended up using it for), so it's possible there's some confusing docs that should be updated? Feel free to point out or PR! Closing this for now!