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

Unnecessary copy of request body in multipart message #838

Open
bgs99 opened this issue Jun 21, 2024 · 1 comment
Open

Unnecessary copy of request body in multipart message #838

bgs99 opened this issue Jun 21, 2024 · 1 comment
Labels
discussion The viability / implementation of the issue is up for debate feature Code based project improvement

Comments

@bgs99
Copy link
Contributor

bgs99 commented Jun 21, 2024

When troubleshooting memory usage of our application using this library, I found that crow::multipart::message stores a copy of the uploaded multipart data that is already present in the crow::request.

This seems unnecessary, as (according to my understanding of the spec (https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html) multipart data is not encoded, so it could simply reference the data in the crow::request.

This would significantly lower memory consumption of the request handlers when dealing with huge multipart file uploads.

Would you accept a string_view-based multipart message/parser to replace or complement the current one?

I have a working prototype that uses std::string_view from C++17 STL that I could contribute.
But as I understand, this library promises C++11 compatibility, so it is necessary to use a different implementation. Would you prefer using some existing string_view implementation or directly re-implementing one inside Crow itself?

@gittiver gittiver added feature Code based project improvement discussion The viability / implementation of the issue is up for debate labels Jun 21, 2024
@gittiver
Copy link
Member

sounds not that it was intended that way.
C++17 discussion is a separate thing, I don't know who wants to decide that.
In my opinion in 2024 we could update to C++14 or 17.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion The viability / implementation of the issue is up for debate feature Code based project improvement
Projects
None yet
Development

No branches or pull requests

2 participants