Fix multipart Content-Type headers with both boundary and charset parameters #1516
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yhirose
reviewed
Mar 8, 2023
httplib.h
Outdated
| @@ -3890,7 +3890,8 @@ inline bool parse_multipart_boundary(const std::string &content_type, | |||
| std::string &boundary) { | |||
| auto pos = content_type.find("boundary="); | |||
| if (pos == std::string::npos) { return false; } | |||
| boundary = content_type.substr(pos + 9); | |||
| auto end = content_type.find(';', pos); | |||
| boundary = content_type.substr(pos + 9, end - pos - 9); | |||
There was a problem hiding this comment.
Could something like below be easier to understand?
auto boundary_keyword = "boundary=";
auto pos = content_type.find(boundary_keyword);
auto beg = pos + strlen(boundary_keyword);
boundary = content_type.substr(beg, end - beg);There was a problem hiding this comment.
Yes, totally. I agree. I will make the modification tomorrow. Thanks for the suggestion.
There was a problem hiding this comment.
Done, and pushed on my branch.
|
@mgaillard could you please take a look at some compile errors on GitHub Actions CI (ubuntu-latest)? Thanks! |
|
I could not reproduce the problem on my side, but I think it has to do with the fact that the build is split into .h and .cpp maybe? I noticed that there was no forward declaration for |
|
@mgaillard thanks for the fine contribution! |
ExclusiveOrange
pushed a commit
to ExclusiveOrange/cpp-httplib-exor
that referenced
this pull request
May 2, 2023
…ameters (yhirose#1516) * Fix multipart Content-Type headers with both boundary and charset parameters * Improve code readability * Add missing forward declaration --------- Co-authored-by: Mathieu Gaillard <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is for Issue #1515.
If the webserver returns a multipart response and include both boundary and charset parameters in the Content-Type header, then only the boundary needs to be read by
parse_multipart_boundary(). I am not sure that including boundary + charset is something that follows the standard, but when querying an openresty server, we get this case.I tried my best to read the RFC standards and do something that works in the general case. According to section 5.1.1 of the RFC2046, the boundary cannot contain a semicolon, so I just search for the first semicolon after the "boundary=" keyword and cut the substring at this position. I added four tests to check that it works.