Fix FSS.S3 support when bucket is not provided #682
Conversation
| hash = | ||
| :crypto.hash(:sha256, entry.bucket <> "/" <> entry.key) |> Base.url_encode64(padding: false) | ||
| :crypto.hash(:sha256, bucket <> "/" <> entry.key) |> Base.url_encode64(padding: false) |
There was a problem hiding this comment.
I would encode the endpoint in the hash too.
lib/explorer/fss/s3.ex
Outdated
| uri = | ||
| if endpoint = entry.config.endpoint do | ||
| URI.parse(endpoint) | ||
| if not is_nil(config.bucket) and not String.contains?(config.endpoint, config.bucket) do |
There was a problem hiding this comment.
Invert the if and else branches to simplify the condition?
|
Hi @philss, unfortunately I am worried of false positives. Imagine you have a bucket named "co" or "zon". Those will both be part of the URL. A better check would be to get the endpoint, split on "." and "/", and then check if it is present, but even that will have a false positive on "com". I would prefer to be explicit. Could we do this instead:
Then, we don't have to guess. Kino will always return virtual style, with no bucket. When parsing "s3://" style URLs, we can do either "s3://bucket/foobar" (the bucket is not in the endpoint) and "s3:///foobar" (virtual host style, the bucket will be in the endpoint). This should support all cases with no ambiguity. Would this work? |
This makes Explorer treat the "endpoint" as already containing the bucket name, and force the virtual host style in order to make Polars ignore the bucket name. Since FSS is always building the correct endpoint in case the user does not provide, we changed the endpoint to be required in the Rust struct. This work is related to elixir-explorer/fss#2
It addresses José's suggestions to simplify the endpoint.
philss
force-pushed
the
ps-handle-s3-entries-without-bucket-name
branch
from
ce54bb2
to
e1eb8d6
Compare
This makes Explorer treat the "endpoint" as already containing the bucket name, and force the virtual host style in order to make Polars ignore the bucket name.
Since FSS is always building the correct endpoint in case the user does not provide, we changed the endpoint to be required in the Rust struct.
This work is related to elixir-explorer/fss#2