[k2] add support multipart/form-data to HTTP server#1423
[k2] add support multipart/form-data to HTTP server#1423nekishdev wants to merge 1 commit intoVKCOM:masterfrom
Conversation
| #include "runtime-common/core/runtime-core.h" | ||
| #include "runtime-common/core/std/containers.h" | ||
| #include "runtime-common/stdlib/server/url-functions.h" | ||
| #include "runtime-common/stdlib/string/string-functions.h" |
There was a problem hiding this comment.
Looks like these includes are actually unused
| f$parse_str(body_str, superglobals.v$_POST); | ||
| } else if (content_type.starts_with(CONTENT_TYPE_MULTIPART_FORM_DATA)) { | ||
| kphp::log::error("unsupported content-type: {}", CONTENT_TYPE_MULTIPART_FORM_DATA); | ||
| std::string_view boundary = parse_boundary(content_type); |
There was a problem hiding this comment.
Minor comments:
- We use braced initialization throughout the codebase. Please, update the initialization to use braces:
std::string_view boundary{parse_boundary};. - Ensure consistency in namespace usage by either consistently applying the
kphp::http::prefix or omitting it across the codebase. Let's only use single approach for clarity and maintainability.
There was a problem hiding this comment.
Please, add missing copyright header
|
|
||
| namespace kphp::http { | ||
|
|
||
| void parse_multipart(const std::string_view &body, const std::string_view &boundary, mixed &v$_POST, mixed &v$_FILES); |
There was a problem hiding this comment.
Actually, there is no need to passing a std::string_view object by reference as it's quite small. Passing by reference introduces an indirection on the other hand
|
|
||
| void parse_multipart(const std::string_view &body, const std::string_view &boundary, mixed &v$_POST, mixed &v$_FILES); | ||
|
|
||
| std::string_view parse_boundary(const std::string_view &content_type); |
There was a problem hiding this comment.
Please, mark all functions noexcept
| constexpr std::string_view HEADER_CONTENT_DISPOSITION_FORM_DATA = "form-data;"; | ||
| constexpr std::string_view MULTIPART_BOUNDARY_EQ = "boundary="; | ||
|
|
||
| struct Header { |
There was a problem hiding this comment.
Just to follow the code style of the runtime-light, let's name types in lowercase
| struct Header { | ||
| std::string_view header_str, name, value; | ||
|
|
||
| Header() {} |
There was a problem hiding this comment.
Please, use = default if you want a trivial constructor
| : header_str{header_str_} {} | ||
|
|
||
| void parse() { | ||
| char delim = ':'; |
There was a problem hiding this comment.
There is vk::split_string_view function. Here is the usage reference: runtime-light/server/http/init-functions.cpp:119
| } | ||
|
|
||
| size_t | ||
| attrStart = header.find_first_not_of(' ', pos), |
There was a problem hiding this comment.
Please, use separate lines to initialize/assign multiple variables
| file[string("size")].push_back(file_size); | ||
| file[string("tmp_name")].push_back(string(tmp_name.data(), tmp_name.size())); | ||
| file[string("error")].push_back(0); | ||
| } else { |
There was a problem hiding this comment.
Is it really possible that file_size < 0?
nekishdev
force-pushed
the
ngoryanoy/k2/http-multipart
branch
2 times, most recently
from
75a4ad4 to
9a96972
Compare
nekishdev
force-pushed
the
ngoryanoy/k2/http-multipart
branch
from
9a96972 to
e22a5e9
Compare
2 participants
No description provided.