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

iostream/http: Fix output_stream::write(temporary_buffer) overload #2436

Merged
merged 2 commits into from
Oct 7, 2024

Conversation

niekbouman
Copy link
Contributor

@niekbouman niekbouman commented Sep 15, 2024

Previously, calling the write(temporary_buffer<char>) overload on an output_stream<char> obtained from http::reply::write_body would cause an abort.

This commit fixes this by providing a helper function (in the data_sink_impl base class) named fallback_put, which classes that inherit from this base class can use to implement the put(net::packet) overload. Also, this commit lets http_chunked_data_sink_impl and http_content_length_data_sink_impl make use of this helper.

fallback_put(net::packet) works simply by extracting the temporary_buffers contained in the packet, and calling the put(temporary_buffer<char>) overload for each temporary_buffer.

Note that we create this fallback_put function instead of defining the virtual member put(net::packet) function in the data_sink_impl class, to avoid put(net::packet) and put(temporary_buffer<char>) calling each other leading to infinite recursion.

This commit also adds unit tests to tests/unit/httpd_test.cc for testing the write overload with http_chunked_data_sink_impl and http_content_length_data_sink_impl.

Thanks to Pavel (xemul) for his help.

Fixes #1701

@niekbouman niekbouman marked this pull request as draft September 16, 2024 13:05
Previously, calling the 'write(temporary_buffer<char>)' overload on an
output_stream<char> obtained from http::reply::write_body
would cause an abort.

This commit fixes this by providing a helper function (in the data_sink_impl
base class) named 'fallback_put', which classes that inherit from this
base class can use to implement the 'put(net::packet)' overload.
Also, this commit lets http_chunked_data_sink_impl and
http_content_length_data_sink_impl make use of this helper.

fallback_put(net::packet) works simply by extracting the temporary_buffers
contained in the packet, and calling the 'put(temporary_buffer<char>)'
overload for each temporary_buffer.

Note that we create this 'fallback_put' function instead of defining the
virtual member 'put(net::packet)' function in the data_sink_impl class,
to avoid 'put(net::packet)' and 'put(temporary_buffer<char>)' calling
each other leading to infinite recursion.

This commit also adds a unit test to tests/unit/httpd_test.cc

Thanks to Pavel (xemul) for his help.

Fixes scylladb#1701
@niekbouman niekbouman marked this pull request as ready for review September 16, 2024 13:26
@niekbouman niekbouman marked this pull request as draft October 6, 2024 10:47
@niekbouman niekbouman marked this pull request as ready for review October 6, 2024 10:50
@niekbouman
Copy link
Contributor Author

@xemul could you please review?

@xemul xemul merged commit 3b2f6f7 into scylladb:master Oct 7, 2024
15 checks passed
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.

In seastar::http::reply::write_body, using output_stream::write(temporary_buffer) causes crash
3 participants