RFC for Attachments

[richo]

Attachments in rocket

This is something of a wip, but I have limited time to work on this so I wanted to get some thoughts down and open it up for comment.

I'm particularly interested in strong opinions about how this should work- I am relatively new to Rocket so I don't have a great sense of how the project prefers various ergonomic options. If someone convincingly tells me what this should look like I will gladly implement it!

Problem:

It's currently very difficult to add a Content-Disposition header to a Response object, since:

• There's no raw access to headers
• There's no wrapping Responder that will do this

Options considered

The naive approach is to implement a wrapping responder, eg:

#[get("/thing")]
fn get_thing() -> Attachment<Content<String>> {
    let content = ...;
    Attachment(Content(content), "thing.ext")
}

This works, although it quickly gets unwieldy if you already have deeply nested responders.

The most sophisticated approach, which unfortunately implies changes to the
underlying data structures is to add .attachment() as a method on Responders
where that makes sense. eg,

#[get("/file")]
fn get_file() -> NamedFile {
  NamedFile::new(File::open(...)).attachment("filename.txt")
}

#[get("/content")]
fn get_content() -> Content {
  Content(...).attachment("filename.txt")
}

The changes all responders to either lack a content-disposition header or set
it to inline implicitly, however the attachment method adds that header.

Unfortunately, I'm not aware of a way to do this without either making Content
(and friends) no longer tuple structs, or going deep on traits and making the
signature instead :

fn get_file() -> impl NamedFile {
  NamedFile::new(File::open(...)), attachment("filename.txt")
}

I have a branch which is ergonomically unpleasant, but verifies that the
underlying idea is workable at
https://github.com/richo/Rocket/tree/wip-attachment

[SergioBenitez]

This works, although it quickly gets unwieldy if you already have deeply nested responders.

Ideally, with the new derive for Responder, deeply nested responders won't be as common.

It's currently very difficult to add a Content-Disposition header to a Response object

I'm wondering if an as_attachment() method on NamedFile wouldn't suffice. You might still need a Content<NamedFile> for your particular use-case it sounds like, but that's better than an Attachment<Content<NamedFile>> responder, and it's a simple solution. Would you find this ergonomic for your use-case?

I'm not sure that emitting an arbitrary Content as an attachment makes sense, actually. It seems easy to misuse as the data being emitted may not represent something suitable to be used as an attachment.

[richo]

So the specific usecase I am looking at right now, the content is built up in memory, so Content handles both attaching a content type and returning the data. I could in theory write it out to disk and then use a NamedFile but that seems a bit strange, as well as I think it'll actually be really hard to make work without leaking those files since I wouldn't be able to lend it a TempFile

I did misinterpret NamedFile's contract, I thought it added the content-disposition header, but it actually only does the mime type (if understood). In some ways it seems like NamedFile is superfluous in a universe with an impl of Content for Read?

I'm not familiar with the new derive for Responder. Where should I go to have a look?

[SergioBenitez]

@richo The documentation for the derive is here. Give it a look. Perhaps it decreases the nesting, making an Attachment wrapping responder suitable.

[richo]

I think at face value it makes the implementation of Attachment that I already have simpler, but doesn't materially change the deep nesting, unless I'm missing something.

[SergioBenitez]

Accepted, and moving to tracking in #95.