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

Improvement to multipart parser #1519

Closed
wants to merge 6 commits into from
Closed

Conversation

mgaillard
Copy link
Contributor

I noticed the parsing of multipart forms had two issues:

  1. The regex would fail if attributes are not in the following order: name, filename, filename*
  2. Filename* attributes are not supported

This PR is my attempt at solving the problem. I noticed two issues are related to this piece of code: #1250 and #1212.

@mgaillard
Copy link
Contributor Author

I see that the build is failing because my tests are not compatible with the .h .cpp split script. I will look at it tomorrow.

@mgaillard
Copy link
Contributor Author

When running split.py the class MultipartFormDataParser ends up in the httplib.cc file, which is not visible in the test.cc
For writing tests it is much easier to access directly MultipartFormDataParser instead of going through a webserver, etc.
Would it be fine to split declaration and definition of MultipartFormDataParser so that it becomes public?

Mathieu Gaillard added 2 commits July 14, 2023 10:31
author Mathieu Gaillard <gaillard@adobe.com> 1678229209 -0800
committer Mathieu Gaillard <gaillard@adobe.com> 1689355104 -0700

Fix multipart Content-Type headers with both boundary and charset parameters

Improve code readability

Add missing forward declaration

Add tests for parsing multipart responses

Change set_boundary to take r-values or l-values

Improve the parsing of attributes name, filename and filename* in the multipart parser

Use lowercase to compare attribute names

Fix typos (yhirose#1517)

Remove useless regex
…answers, and allow testing of MultipartFormDataParser.
@mgaillard
Copy link
Contributor Author

I added:

bool parse_multipart_response(const Response &res, MultipartFormDataItems& files);

To allow testing the multipart form data parser without making it public.
It also allows the client code to parse multipart responses from the server.

@mgaillard
Copy link
Contributor Author

I fixed the code so that tests pass, could I get a review?

@yhirose yhirose closed this in aabf752 Jul 29, 2023
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.

None yet

2 participants