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

Create a method to write a multipart entity on an HTTPClientRequest. #33

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vnayar
Copy link
Contributor

@vnayar vnayar commented Mar 14, 2024

See #32

This is an initial commit to sketch out the work and get early feedback. The author is unfamiliar with Vibe's internals and conventions, thus iteration and feedback will be needed.

Depends on: vibe-d/vibe-inet#2

Copy link
Member

@s-ludwig s-ludwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this will be the >90% case, there should be an additional way to write a multipart form in a more direct way, such as like mentioned in #32 (comment) - this/these would probably just be overloads of writeFormBody here, to keep it short.


See_Also: https://datatracker.ietf.org/doc/html/rfc2388
*/
void writeMultipartEntity(MultipartEntity entity)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more consistent with the other methods, I'd suggest writeMultiPartBody as the name ("body" instead of "entity" makes it clear that this is the whole body and not just one of possibly multiple pieces and "MultiPart" is the naming convention used in vibe.inet.webform)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the long hiatus, lots of stuff got in the way. But resuming where we left off...

So there's pros and cons to both approaches in my view, although I'm fine with either:

Appraoch 1: Request has headers + body

  • Matches current code base.
  • More intuitive for the casual user.
  • Unclear when introducing multipart, as the multipart also has headers and body.

Approach 2: Request has entity, which has headers + body

  • Doesn't match current code conventions in this library.

Roughly by RFC spec, what we have is (example form request with 2 fields, simple text, and multiple-files):

  • HTTP Request
    • Entity (Multipart Entity of type multipart/form-data)
      • Headers (Including boundary and multipart/form-data headers)
      • Body
        • Multipart Part 1 (Entity w/ headers + body)
          • Part 1 Headers (content-disposition, etc.)
          • Part 1 Body
            • Text, Checkbox, Single image, etc.
        • Top Multipart Boundary
        • Multipart Part 2 Entity (Multiple files https://datatracker.ietf.org/doc/html/rfc2388#section-4.2)
          • Part 1 Headers (with Contant-Type: multipart/mixed with own boundary)
          • Part 1 Body
            • Multipart Part 2a Entity
              • Headers (first file)
              • Body 2a
            • Field Multipart Boundary
            • Multipart Part 2b Entity
              • Headers (second file)
              • Body 2b
            • Field Multipart Boundary End
        • Form Multipart Boundary End

While the full MIME multipart spec can form an arbitrary tree-like structure using the multipart/mixed or multipart/alternative. https://datatracker.ietf.org/doc/html/rfc2046 However, with multipart/form-data, we're basically limited to a tree with 2 levels past the root (the list of form fields, and some fields can contain multiple files using multipart/mixed). The current code for vibe.http.common.MultiPart doesn't seem to handle this case.

With all that being said, I think the following naming conventions would translate between the two worlds:

  • The term "body" is used only for the HTTP Request entity body.
  • If the HTTP Request entity body is a Multipart Entity, that is called MultiPartBody, and it contains headers and a body of MultiPart[].
  • The entity (headers + body) inside a multipart body part is referred to as MultiPart, which can contain:
    1. An entity for a form value, typically only with the header Content-Disposition (with the field name) and a simple text body, represented as string (assuming headers are absent or the caller will ignore them).
    2. An entity for a form value as a single file, with the Content-Disposition (with the field name and suggested filename), Content-Type, and other headers.
    3. An entity for multiple files, with Content-Type set to multipart/mixed, represented by MultiPart[]

As time permits, I'll start building this up and hopefully handle some more cases, to at least achieve parity with libraries like Apache HTTP Client: https://www.tutorialspoint.com/apache_httpclient/apache_httpclient_multipart_upload.htm

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note, does it matter that the writeMultiPartBody will also write headers? Maybe just writeMultiPart?


auto dst = bodyWriter(); // This method finalizes headers when called.

// Write the individual multipart parts.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code that follows here should be in a new function vibe.inet.webform.writeMultiPartEntity.

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

Successfully merging this pull request may close these issues.

2 participants