[k2] add support http multipart content-type by astrophysik · Pull Request #1545 · VKCOM/kphp

astrophysik · 2026-03-06T13:54:23Z

No description provided.

runtime-light/server/http/init-functions.cpp

runtime-light/server/http/multipart/details/parts-processing.cpp

apolyakov · 2026-03-12T13:41:34Z

runtime-light/server/http/init-functions.cpp

 #include "runtime-light/state/instance-state.h"
 #include "runtime-light/stdlib/component/component-api.h"
 #include "runtime-light/stdlib/diagnostics/logs.h"
+#include "runtime-light/stdlib/file/file-system-functions.h"


unused include

apolyakov · 2026-03-12T13:41:41Z

runtime-light/server/http/init-functions.cpp

 #include "runtime-light/tl/tl-functions.h"
 #include "runtime-light/tl/tl-types.h"

+#include "runtime-light/stdlib/output/print-functions.h"


unused include

apolyakov · 2026-03-12T13:45:11Z

runtime-light/server/http/init-functions.cpp

      http_server_instance_st.opt_raw_post_data.emplace(std::move(body));
    } else if (!std::ranges::search(content_type, CONTENT_TYPE_MULTIPART_FORM_DATA).empty()) {
-      kphp::log::error("unsupported content-type: {}", CONTENT_TYPE_MULTIPART_FORM_DATA);
+      if (auto boundary_opt{kphp::http::multipart::extract_boundary(content_type)}; boundary_opt.has_value()) {


Looks like an error that we do nothing in case we couldn't extract boundary

Add warning in this case

apolyakov · 2026-03-12T13:46:31Z

runtime-light/server/http/init-functions.cpp


 kphp::coro::task<> finalize_server() noexcept {
  auto& http_server_instance_st{HttpServerInstanceState::get()};
+  auto& superglobals{InstanceState::get().php_script_mutable_globals_singleton.get_superglobals()};


This variable is only used in a single switch-case. Let's declare it there to not waste CPU cycles unless it's really necessary

nit: you may use PhpScriptMutableGlobals::current() instead of InstanceState::get().php_script_mutable_globals_singleton

apolyakov · 2026-03-12T13:47:29Z

runtime-light/server/http/init-functions.cpp

+    const array<mixed> files{superglobals.v$_FILES.to_array()};
+    for (const auto& files_it : files) {
+      const mixed& file{files_it.get_value()};
+      const mixed& tmp_filenames{file.get_value(string{"tmp_name"})};


string{"tmp_name"} constructs a new string object every iteration. Since this is a constant key, it could be cached

apolyakov · 2026-03-12T14:24:05Z

runtime-light/server/http/multipart/details/parts-processing.cpp

+  return std::nullopt;
+}
+
+std::expected<size_t, int32_t> write_temporary_file(std::string_view tmp_name, std::span<const std::byte> content) noexcept {


Consider following implementation:

std::expected<size_t, int32_t> write_temporary_file(std::string_view tmp_name, std::span<const std::byte> content) noexcept { auto file_res{kphp::fs::file::open(tmp_name, "w")}; if (!file_res.has_value()) { return std::unexpected{UPLOAD_ERR_NO_FILE}; } auto written_res{(*file_res).write(content)}; if (!written_res.has_value()) { return std::unexpected{UPLOAD_ERR_CANT_WRITE}; } size_t file_size{*written_res}; if (file_size < content.size()) { return std::unexpected{UPLOAD_ERR_PARTIAL}; } return file_size; }

Moreover, I don't see a reason to return std::expected<size_t, int32_t> instead of std::expected<void, size_t>

Thanks
Error code and size are used in mixed file{}; initialization

apolyakov · 2026-03-12T14:25:30Z

runtime-light/server/http/multipart/details/parts-processing.cpp

+  const string name{part.name_attribute.data(), static_cast<string::size_type>(part.name_attribute.size())};
+  const string body{part.body.data(), static_cast<string::size_type>(part.body.size())};
+  if (part.content_type.has_value() && !std::ranges::search(*part.content_type, CONTENT_TYPE_APP_FORM_URLENCODED).empty()) {
+    f$parse_str(body, post[name]);


It'd be more systematic to use get_value here

Okay, but it leads to extra allocation

apolyakov · 2026-03-12T14:26:10Z

runtime-light/server/http/multipart/details/parts-processing.cpp

+void process_post_multipart(const kphp::http::multipart::details::part& part, mixed& post) noexcept {
+  const string name{part.name_attribute.data(), static_cast<string::size_type>(part.name_attribute.size())};
+  const string body{part.body.data(), static_cast<string::size_type>(part.body.size())};
+  if (part.content_type.has_value() && !std::ranges::search(*part.content_type, CONTENT_TYPE_APP_FORM_URLENCODED).empty()) {


Why not *part.content_type == CONTENT_TYPE_APP_FORM_URLENCODED?

Because content-type may looks like Content-type: application/x-www-form-urlencoded; charset=UTF-8

apolyakov · 2026-03-12T14:29:09Z

runtime-light/server/http/multipart/details/parts-processing.cpp

+
+  auto tmp_name_opt{generate_temporary_name()};
+  if (!tmp_name_opt.has_value()) {
+    kphp::log::warning("cannot generate unique name for multipart temporary file");


Isn't it dangerous that we can't create a file and just emit a warning? It looks like this may lead to some state corruption

apolyakov · 2026-03-12T14:31:26Z

runtime-light/server/http/multipart/details/parts-processing.cpp

+  auto tmp_name{*tmp_name_opt};
+  auto write_res{write_temporary_file(tmp_name, {reinterpret_cast<const std::byte*>(part.body.data()), part.body.size()})};
+
+  mixed file{};


Isn't it simpler to declare it as array<mixed>?

nekishdev and others added 14 commits March 4, 2026 12:46

[k2] add support multipart/form-data to HTTP server

6d5fd39

[k2] refactor multipart/form-data via ranges

260040e

wip

7df626c

wip

4e5e7f4

wip

84d6505

wip

0fba8b8

wip

4092329

wip

2c4f021

add tests

0f5f1b8

small fix

3f5932a

small fix

67a453e

small fix

3b6cc43

small fix

5cce84c

small fix

822ddad

astrophysik added this to the next milestone Mar 6, 2026

astrophysik self-assigned this Mar 6, 2026

astrophysik added the k2 k2 related label Mar 6, 2026

astrophysik requested review from PetrShumilov, Shamzik and apolyakov March 6, 2026 13:54

astrophysik added 2 commits March 6, 2026 16:55

small fix

49301a0

small fix

c6e2825

astrophysik mentioned this pull request Mar 6, 2026

Ngoryanoy/k2/http multipart via ranges #1538

Closed

apolyakov requested changes Mar 6, 2026

View reviewed changes

astrophysik added 6 commits March 11, 2026 15:44

apply review

5d96fad

add test assert

ca69dde

fix format

97c9931

fix format

920e25e

apply review

279724e

fix format

3b93247

astrophysik added 2 commits March 12, 2026 12:32

remove extra const

3391f1f

add include

03d1809

apolyakov reviewed Mar 12, 2026

View reviewed changes

astrophysik added the enhancement New feature or request label Mar 13, 2026

astrophysik added 2 commits March 13, 2026 17:27

apply review

a92ac9e

fix format

1defac9

Conversation

astrophysik commented Mar 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants