Skip to content
New issue

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

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

Already on GitHub? # to your account

Overwrite Reponse headers for presigned URL #1726

Closed
baszalmstra opened this issue Mar 21, 2023 · 16 comments
Closed

Overwrite Reponse headers for presigned URL #1726

baszalmstra opened this issue Mar 21, 2023 · 16 comments

Comments

@baszalmstra
Copy link
Contributor

Im building a content addressable store where files in storage are named after their content hash. When requesting a certain file from our service we look up the content hash and create a pre-signed URL with opendal based on the hash. However, when Im then redirected the downloaded file will be named the same as the hash. I could modify the metadata of the objects in the object store to include the "content-disposition" but that slightly defeats the purpose of the content-addressable object store.

AWS supports response-content-disposition as well as some other headers to control the headers returned from the redirect. Is that also something opendal could support when using presign_read?

@Xuanwo
Copy link
Member

Xuanwo commented Mar 22, 2023

This feature request is reasonable. I will consider how to incorporate it with other features.

@ClSlaid
Copy link
Contributor

ClSlaid commented Mar 22, 2023

We currently provide a with_content_disposition option for OpWrite, this allows you writing along with a Content-Disposition header. Can this cover your need? 👀️

https://docs.rs/opendal/latest/opendal/ops/struct.OpWrite.html#method.with_content_disposition

This may not be useful if you have multiple names for the same file.

@Xuanwo
Copy link
Member

Xuanwo commented Mar 22, 2023

Yes, they can. But I think there are different feature request.

@baszalmstra
Copy link
Contributor Author

Well yes partially. I can have different files with the same content but with different filenames.

But Im also using the Writer api because Im uploading pretty big files, but unfortunately when using a writer there doesnt seem to be a way to specify the OpWrite. Or did I miss that?

@Xuanwo
Copy link
Member

Xuanwo commented Mar 22, 2023

But Im also using the Writer api because Im uploading pretty big files, but unfortunately when using a writer there doesnt seem to be a way to specify the OpWrite. Or did I miss that?

Oh, yes. We should support writer_with too.

@baszalmstra
Copy link
Contributor Author

That would be great!

@baszalmstra
Copy link
Contributor Author

@Xuanwo is there anything I can help with?

@Xuanwo
Copy link
Member

Xuanwo commented Mar 23, 2023

@Xuanwo is there anything I can help with?

Thanks a lot!

There are two things we need to do:

Add writer_with for Operator

OpWrite already supports with_content_disposition, but we only expose them in write_with.

We can add a writer_with with allow users to specify the content_disposition during write.

Add response_content_disposition support in OpRead

I am currently not confident about this feature and require some time to reconsider it.

@baszalmstra
Copy link
Contributor Author

Thanks I will take a look at "Add writer_with for Operator".

Another use case for the response stuff is that you can also specify the cache-control header (at least for s3). Given that my data is stored in an immutable fashion it would be nice to be able to communicate that to the browser/user as well. But maybe we can also add that to OpWrite?

@Xuanwo
Copy link
Member

Xuanwo commented Mar 23, 2023

I am currently not confident about this feature and require some time to reconsider it.

I have an idea on how to handle them. I will initiate a RFC for this.

But maybe we can also add that to OpWrite?

Yes. Let's disscuss it in the RFC.

@baszalmstra
Copy link
Contributor Author

@Xuanwo With #1739 merged would you be open to adding additional headers? S3 supports these:

  • response-content-type
  • response-content-language
  • response-expires
  • response-cache-control
  • response-content-disposition
  • response-content-encoding

I'm mostly interested in response-cache-control. I can also do this by using the OpWrite::cache_control as proposed in #1735 but for my use-case being able to do this per request would be kinda nice.

@Xuanwo
Copy link
Member

Xuanwo commented Mar 30, 2023

I'm mostly interested in response-cache-control

Let's add it!

@baszalmstra
Copy link
Contributor Author

Done in #1804

@Xuanwo
Copy link
Member

Xuanwo commented Mar 30, 2023

Done in #1804

So quick!

@baszalmstra
Copy link
Contributor Author

baszalmstra commented Mar 30, 2023

Although I have what I need which additional headers do you think we should add to close this PR?

  • response-content-type
  • response-content-language
  • response-expires
  • response-content-encoding

Id be happy to add them.

@Xuanwo
Copy link
Member

Xuanwo commented Mar 30, 2023

Although I have what I need which additional headers

Let's prioritize closing this issue first. We can add new features as users request them.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants