Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 19 additions & 39 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,13 @@ jobs:
timeout-minutes: 120

steps:
- name: Clone Boost.Corosio
- name: Clone Boost.Http
uses: actions/checkout@v4
with:
path: http-root

- name: Setup C++
uses: alandefreitas/cpp-actions/setup-cpp@v1.9.0
uses: alandefreitas/cpp-actions/setup-cpp@v1.9.3
id: setup-cpp
with:
compiler: ${{ matrix.compiler }}
Expand All @@ -257,7 +257,7 @@ jobs:
trace-commands: true

- name: Install packages
uses: alandefreitas/cpp-actions/package-install@v1.9.0
uses: alandefreitas/cpp-actions/package-install@v1.9.3
id: package-install
with:
apt-get-add-architecture: ${{ matrix.x86 && 'i386' || '' }}
Expand All @@ -267,22 +267,16 @@ jobs:
zlib1g-dev libbrotli-dev
${{ matrix.x86 && 'zlib1g-dev:i386 libbrotli-dev:i386' || '' }}

- name: Clone Capy
uses: actions/checkout@v4
with:
repository: cppalliance/capy
ref: ${{ (github.ref_name == 'master' && github.ref_name) || 'develop' }}
path: capy-root

- name: Clone Boost
uses: alandefreitas/cpp-actions/boost-clone@v1.9.0
uses: alandefreitas/cpp-actions/boost-clone@v1.9.3
id: boost-clone
with:
branch: ${{ (github.ref_name == 'master' && github.ref_name) || 'develop' }}
boost-dir: boost-source
modules-exclude-paths: ''
modules-exclude-paths: '-'
scan-modules-dir: http-root
scan-modules-ignore: http,capy
patches: >
https://github.com/cppalliance/capy

- name: ASLR Fix
if: ${{ startsWith(matrix.runs-on, 'ubuntu' )}}
Expand All @@ -308,7 +302,6 @@ jobs:

# Remove module from boost-source
rm -r "boost-source/libs/$module" || true
rm -r "boost-source/libs/capy" || true

# Copy cached boost-source to an isolated boost-root
cp -r boost-source boost-root
Expand All @@ -322,11 +315,8 @@ jobs:
# Patch boost-root with workspace module
cp -r "$workspace_root"/http-root "libs/$module"

# Patch boost-root with capy dependency
cp -r "$workspace_root"/capy-root "libs/capy"

- name: Boost B2 Workflow
uses: alandefreitas/cpp-actions/b2-workflow@v1.9.0
uses: alandefreitas/cpp-actions/b2-workflow@v1.9.3
if: ${{ !matrix.coverage }}
env:
ASAN_OPTIONS: ${{ ((matrix.compiler == 'apple-clang' || matrix.compiler == 'clang') && 'detect_invalid_pointer_pairs=0:strict_string_checks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1') || 'detect_invalid_pointer_pairs=2:strict_string_checks=1:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1' }}
Expand All @@ -346,7 +336,7 @@ jobs:
stop-on-error: true

- name: Boost CMake Workflow
uses: alandefreitas/cpp-actions/cmake-workflow@v1.9.0
uses: alandefreitas/cpp-actions/cmake-workflow@v1.9.3
if: ${{ matrix.coverage || matrix.build-cmake || matrix.is-earliest }}
with:
source-dir: boost-root
Expand Down Expand Up @@ -386,7 +376,7 @@ jobs:
# This causes find_package(Boost COMPONENTS http) to fail because
# boost_httpConfig.cmake is never installed.
- name: Find Package Integration Workflow
uses: alandefreitas/cpp-actions/cmake-workflow@v1.9.0
uses: alandefreitas/cpp-actions/cmake-workflow@v1.9.3
if: false # ${{ matrix.build-cmake || matrix.is-earliest }}
with:
source-dir: boost-root/libs/${{ steps.patch.outputs.module }}/test/cmake_test
Expand All @@ -409,7 +399,7 @@ jobs:
trace-commands: true

- name: Subdirectory Integration Workflow
uses: alandefreitas/cpp-actions/cmake-workflow@v1.9.0
uses: alandefreitas/cpp-actions/cmake-workflow@v1.9.3
if: ${{ matrix.build-cmake || matrix.is-earliest }}
with:
source-dir: boost-root/libs/${{ steps.patch.outputs.module }}/test/cmake_test
Expand All @@ -429,7 +419,7 @@ jobs:
ref-source-dir: boost-root/libs/http/test/cmake_test

- name: Root Project CMake Workflow
uses: alandefreitas/cpp-actions/cmake-workflow@v1.9.0
uses: alandefreitas/cpp-actions/cmake-workflow@v1.9.3
if: ${{ matrix.build-cmake || matrix.is-earliest }}
with:
source-dir: boost-root/libs/${{ steps.patch.outputs.module }}
Expand Down Expand Up @@ -492,14 +482,14 @@ jobs:
timeout-minutes: 120

steps:
- name: Clone Boost.Corosio
- name: Clone Boost.Http
uses: actions/checkout@v4
with:
# Avoid the common API rate limit exceeded error in boostorg by including 100 latest commits in any case
fetch-depth: 100

- name: Changelog
uses: alandefreitas/cpp-actions/create-changelog@v1.9.0
uses: alandefreitas/cpp-actions/create-changelog@v1.9.3
with:
thank-non-regular: ${{ startsWith(github.ref, 'refs/tags/') }}
github-token: ${{ secrets.GITHUB_TOKEN }}
Expand All @@ -517,7 +507,7 @@ jobs:
shell: bash
steps:
- name: Install packages
uses: alandefreitas/cpp-actions/package-install@v1.9.0
uses: alandefreitas/cpp-actions/package-install@v1.9.3
with:
apt-get: git cmake

Expand All @@ -526,22 +516,16 @@ jobs:
with:
path: http-root

- name: Clone Capy
uses: actions/checkout@v4
with:
repository: cppalliance/capy
ref: ${{ (github.ref_name == 'master' && github.ref_name) || 'develop' }}
path: capy-root

- name: Clone Boost
uses: alandefreitas/cpp-actions/boost-clone@v1.9.0
uses: alandefreitas/cpp-actions/boost-clone@v1.9.3
id: boost-clone
with:
branch: ${{ (github.ref_name == 'master' && github.ref_name) || 'develop' }}
boost-dir: boost-source
modules-exclude-paths: ''
modules-exclude-paths: '-'
scan-modules-dir: http-root
scan-modules-ignore: http,capy
patches: >
https://github.com/cppalliance/capy

- name: Patch Boost
id: patch
Expand All @@ -562,7 +546,6 @@ jobs:

# Remove module from boost-source
rm -r "boost-source/libs/$module" || true
rm -r "boost-source/libs/capy" || true

# Copy cached boost-source to an isolated boost-root
cp -r boost-source boost-root
Expand All @@ -576,9 +559,6 @@ jobs:
# Patch boost-root with workspace module
cp -r "$workspace_root"/http-root "libs/$module"

# Patch boost-root with capy dependency
cp -r "$workspace_root"/capy-root "libs/capy"

- uses: actions/setup-node@v4
with:
node-version: 18
Expand Down
23 changes: 11 additions & 12 deletions include/boost/http/bcrypt.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,15 @@
#include <boost/system/error_code.hpp>
#include <boost/system/is_error_code_enum.hpp>

#include <boost/capy/coro.hpp>
#include <boost/capy/task.hpp>
#include <boost/capy/ex/executor_ref.hpp>
#include <boost/capy/ex/io_env.hpp>
#include <boost/capy/ex/run_async.hpp>
#include <boost/capy/ex/system_context.hpp>

#include <cstddef>
#include <cstring>
#include <exception>
#include <stop_token>
#include <string>
#include <system_error>

Expand Down Expand Up @@ -532,24 +531,24 @@ struct hash_async_op
}

void await_suspend(
capy::coro cont,
capy::executor_ref caller_ex,
std::stop_token)
std::coroutine_handle<void> cont,
capy::io_env const* env)
{
auto caller_ex = env->executor;
auto& pool = capy::get_system_context();
auto sys_ex = pool.get_executor();
capy::run_async(sys_ex,
[this, cont, caller_ex]
(result r) mutable
{
result_ = r;
caller_ex.dispatch(cont);
caller_ex.dispatch(cont).resume();
},
[this, cont, caller_ex]
(std::exception_ptr ep) mutable
{
ep_ = ep;
caller_ex.dispatch(cont);
caller_ex.dispatch(cont).resume();
}
)(hash_task(password_, rounds_, ver_));
Comment on lines 533 to 553
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, check if the file exists and read the relevant sections
wc -l include/boost/http/bcrypt.hpp

Repository: cppalliance/http

Length of output: 93


🏁 Script executed:

#!/bin/bash
# Read lines around 533-553 (await_suspend) and 580 (compare_async_op)
sed -n '530,560p' include/boost/http/bcrypt.hpp | cat -n

Repository: cppalliance/http

Length of output: 1103


🏁 Script executed:

#!/bin/bash
# Check lines around 580 for compare_async_op
sed -n '575,600p' include/boost/http/bcrypt.hpp | cat -n

Repository: cppalliance/http

Length of output: 956


🏁 Script executed:

#!/bin/bash
# Search for the function signatures and any documentation about io_env
rg -n 'await_suspend|compare_async_op' include/boost/http/bcrypt.hpp -B 3 -A 1

Repository: cppalliance/http

Length of output: 638


🏁 Script executed:

#!/bin/bash
# Search for io_env usage patterns to understand if it can be null
rg -n 'io_env' include/boost/http/bcrypt.hpp -B 2 -A 2

Repository: cppalliance/http

Length of output: 602


🏁 Script executed:

#!/bin/bash
# Search for how hash_async and compare_async are used (call sites)
rg -n 'hash_async|compare_async' include/boost/http/bcrypt.hpp -A 5

Repository: cppalliance/http

Length of output: 1624


🏁 Script executed:

#!/bin/bash
# Look for any co_await usage in the codebase to understand context
rg -n 'co_await' . --type=cpp | head -20

Repository: cppalliance/http

Length of output: 1777


🏁 Script executed:

#!/bin/bash
# Check if there are other await_suspend implementations for comparison
rg -n 'await_suspend' . --type=cpp -B 3 -A 8 | head -50

Repository: cppalliance/http

Length of output: 1594


🏁 Script executed:

#!/bin/bash
# Search for io_env usage patterns and whether it can be null
rg -n 'io_env' . --type=cpp | grep -E '(null|nullptr|if|check)' | head -20

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Look for any guard conditions around await_suspend or env parameter
fd -e hpp -e cpp | xargs grep -l 'await_suspend' | head -5

Repository: cppalliance/http

Length of output: 89


🏁 Script executed:

#!/bin/bash
# Search for test usage of hash_async and compare_async
fd -e cpp test | xargs grep -l 'hash_async\|compare_async'

Repository: cppalliance/http

Length of output: 44


🏁 Script executed:

#!/bin/bash
# Check bcrypt tests to see context where these are called
find . -path '*test*' -name '*bcrypt*' -type f

Repository: cppalliance/http

Length of output: 82


🏁 Script executed:

#!/bin/bash
# Search for any documentation or other uses of io_env parameter contract
rg -n 'io_env' . --type=cpp -B 2 -A 2 | grep -E '(null|nullptr|guarantee|contract|non-null)' -i

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Look for how capy framework is documented or used
fd -e hpp include/boost/capy | head -10

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Check if capy provides any guard or context management for io_env
rg -n 'io_env' include/boost/capy --type=hpp -A 3 | head -40

Repository: cppalliance/http

Length of output: 87


🏁 Script executed:

#!/bin/bash
# Read the bcrypt test file to understand usage context
cat test/unit/bcrypt.cpp

Repository: cppalliance/http

Length of output: 15438


🏁 Script executed:

#!/bin/bash
# Search for any co_await context or io_env setup in tests
rg -n 'hash_async|compare_async' test/ -B 10 -A 5

Repository: cppalliance/http

Length of output: 5648


🏁 Script executed:

#!/bin/bash
# Check if there are any other two-parameter await_suspend implementations
rg -n 'await_suspend' . --type=cpp -B 5 | grep -E '(await_suspend|const.*\*|nullptr|null)' | head -40

Repository: cppalliance/http

Length of output: 179


🏁 Script executed:

#!/bin/bash
# Search for capy documentation or examples about await_suspend contract
find . -name "*.hpp" -o -name "*.cpp" | xargs grep -l 'await_suspend' | xargs grep -l 'capy'

Repository: cppalliance/http

Length of output: 91


🏁 Script executed:

#!/bin/bash
# Look for any capy framework notes about io_env requirements
rg -n 'io_env' . --type=cpp --type=hpp | grep -E '(nullptr|null|require|guarantee|contract|must|non-null)' -i

Repository: cppalliance/http

Length of output: 87


🏁 Script executed:

#!/bin/bash
# Check if there are defensive null checks in similar code patterns
rg -n 'if.*env' include/boost/http/ --type=cpp --type=hpp

Repository: cppalliance/http

Length of output: 87


🏁 Script executed:

#!/bin/bash
# Search for web references or external docs about capy await_suspend contract
grep -r "capy" README.md doc/ 2>/dev/null | head -20

Repository: cppalliance/http

Length of output: 1967


🏁 Script executed:

#!/bin/bash
# Check if there's any CMakeLists or dependency info about capy
find . -name "CMakeLists.txt" -o -name "*.txt" | xargs grep -i capy 2>/dev/null

Repository: cppalliance/http

Length of output: 433


🏁 Script executed:

#!/bin/bash
# Look for any comments or documentation near the await_suspend definitions
sed -n '500,540p' include/boost/http/bcrypt.hpp

Repository: cppalliance/http

Length of output: 943


🏁 Script executed:

#!/bin/bash
# Look for the complete struct definitions and any surrounding documentation
sed -n '520,562p' include/boost/http/bcrypt.hpp

Repository: cppalliance/http

Length of output: 1079


🏁 Script executed:

#!/bin/bash
# Check the second function as well
sed -n '564,605p' include/boost/http/bcrypt.hpp

Repository: cppalliance/http

Length of output: 1069


🏁 Script executed:

#!/bin/bash
# Search for any similar two-parameter await_suspend in other Boost libraries
find . -name "*.hpp" -o -name "*.cpp" | xargs grep -l 'await_suspend' 2>/dev/null

Repository: cppalliance/http

Length of output: 91


🏁 Script executed:

#!/bin/bash
# Search for capy::io_env definition or contract information
find . -name "io_env.hpp" -o -name "*io_env*" 2>/dev/null | head -5

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Check if capy is vendored or an external dependency
ls -la include/boost/capy/ 2>/dev/null | head -20

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Look for any documentation or comments about two-parameter await_suspend
sed -n '500,530p' include/boost/http/bcrypt.hpp

Repository: cppalliance/http

Length of output: 667


🏁 Script executed:

#!/bin/bash
# Look at the public API documentation for hash_async and compare_async
sed -n '610,650p' include/boost/http/bcrypt.hpp

Repository: cppalliance/http

Length of output: 1121


🏁 Script executed:

#!/bin/bash
# Check if there's any `@requires` or precondition documentation
sed -n '600,680p' include/boost/http/bcrypt.hpp

Repository: cppalliance/http

Length of output: 1999


🏁 Script executed:

#!/bin/bash
# Search for any documentation about capy::io_env contract or preconditions
grep -r "io_env" doc/ README.md 2>/dev/null | grep -i "require\|precond\|contract\|must\|non-null" -i

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Check if there are other async operations in the codebase with similar patterns
find . -name "*.hpp" -o -name "*.cpp" | xargs grep -l 'await_suspend' 2>/dev/null | xargs grep -A 5 'await_suspend'

Repository: cppalliance/http

Length of output: 450


🏁 Script executed:

#!/bin/bash
# Look for error handling patterns or null checks in capy usage
rg -n 'env' include/boost/http/ -A 2 | grep -E '(nullptr|if|check)' | head -20

Repository: cppalliance/http

Length of output: 42


Add null-check or document the requirement that these awaitables must be co_await'd only within a capy io_env context.

env->executor at lines 537 and 580 dereferences env without validation. While the API is designed for capy coroutines where the framework provides io_env, there is no documented precondition preventing misuse. If either hash_async or compare_async awaitables are co_await'd outside a capy context, env could be null, causing undefined behavior. Either add a null-check with a descriptive error, or prominently document that these awaitables require execution within a capy io_env context.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/boost/http/bcrypt.hpp` around lines 533 - 553, The await_suspend
method currently dereferences env (env->executor) without validation, which can
crash if the awaitable is used outside a capy io_env; in await_suspend (the
lambda using caller_ex and the calls to caller_ex.dispatch(cont).resume()) add a
null-check for env (or env->executor) and handle the error by capturing/setting
ep_ to a descriptive std::runtime_error (or call cont.resume on the caller with
an exception) so misuse fails fast and clearly, or alternatively document in the
header that await_suspend (and the hash_async/compare_async awaitables that call
hash_task/compare_task) require being co_awaited inside a capy io_env. Ensure
the check references await_suspend, env->executor, and the
hash_task/compare_task call sites so reviewers can locate the change.

}
Expand All @@ -575,24 +574,24 @@ struct compare_async_op
}

void await_suspend(
capy::coro cont,
capy::executor_ref caller_ex,
std::stop_token)
std::coroutine_handle<void> cont,
capy::io_env const* env)
{
auto caller_ex = env->executor;
auto& pool = capy::get_system_context();
auto sys_ex = pool.get_executor();
capy::run_async(sys_ex,
[this, cont, caller_ex]
(bool ok) mutable
{
result_ = ok;
caller_ex.dispatch(cont);
caller_ex.dispatch(cont).resume();
},
[this, cont, caller_ex]
(std::exception_ptr ep) mutable
{
ep_ = ep;
caller_ex.dispatch(cont);
caller_ex.dispatch(cont).resume();
}
)(compare_task(password_, hash_str_));
}
Expand Down
30 changes: 19 additions & 11 deletions src/server/serve_static.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,28 +297,36 @@ operator()(route_params& rp) const
// Calculate how much to send
std::int64_t remaining = info.range_end - info.range_start + 1;

// Stream file content
constexpr std::size_t buf_size = 16384;
char buffer[buf_size];

// Stream file content using serializer's internal buffer
while(remaining > 0)
{
capy::mutable_buffer arr[1];
auto bufs = rp.res_body.prepare(arr);
if(bufs.empty())
{
auto [ec2] = co_await rp.res_body.commit(0);
if(ec2)
co_return route_error(ec2);
continue;
}
Comment on lines +303 to +311
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "*.hpp" -o -name "*.cpp" | head -20

Repository: cppalliance/http

Length of output: 822


🏁 Script executed:

fd -e hpp -e cpp "serve_static" | head -10

Repository: cppalliance/http

Length of output: 164


🏁 Script executed:

git ls-files | grep -E "(serve_static|capy)" | head -20

Repository: cppalliance/http

Length of output: 164


🏁 Script executed:

wc -l src/server/serve_static.cpp

Repository: cppalliance/http

Length of output: 91


🏁 Script executed:

sed -n '290,340p' src/server/serve_static.cpp

Repository: cppalliance/http

Length of output: 1297


🏁 Script executed:

rg -n "prepare\s*\(" include/ --type cpp -B 2 -A 10 | head -80

Repository: cppalliance/http

Length of output: 4922


🏁 Script executed:

rg -n "commit\s*\(" include/ --type cpp -B 2 -A 10 | head -80

Repository: cppalliance/http

Length of output: 4789


🏁 Script executed:

sed -n '370,400p' include/boost/http/serializer.hpp

Repository: cppalliance/http

Length of output: 834


🏁 Script executed:

sed -n '620,680p' include/boost/http/serializer.hpp

Repository: cppalliance/http

Length of output: 1630


🏁 Script executed:

rg -n "stream_prepare" include/boost/http/ -B 2 -A 15 | head -100

Repository: cppalliance/http

Length of output: 6486


🏁 Script executed:

grep -n "need_data" include/boost/http/error.hpp

Repository: cppalliance/http

Length of output: 77


🏁 Script executed:

sed -n '620,650p' include/boost/http/serializer.hpp

Repository: cppalliance/http

Length of output: 911


🏁 Script executed:

rg -n "prepare.*empty" include/boost/http/ --type cpp -B 3 -A 3

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

sed -n '655,680p' include/boost/http/serializer.hpp

Repository: cppalliance/http

Length of output: 685


🏁 Script executed:

rg -n "while.*remaining" src/server/serve_static.cpp -B 5 -A 30

Repository: cppalliance/http

Length of output: 1237


🏁 Script executed:

rg -n "commit.*0" test/ --type cpp -B 3 -A 3 | head -60

Repository: cppalliance/http

Length of output: 3037


🏁 Script executed:

sed -n '645,680p' include/boost/http/serializer.hpp

Repository: cppalliance/http

Length of output: 907


🏁 Script executed:

rg -n "stream_capacity" include/boost/http/serializer.hpp -B 2 -A 10

Repository: cppalliance/http

Length of output: 1684


🏁 Script executed:

rg -n "stream_prepare\|stream_commit" src/ include/ --type cpp -B 2 -A 8 | grep -A 8 -B 2 "empty"

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

sed -n '1,100p' include/boost/http/serializer.hpp | head -60

Repository: cppalliance/http

Length of output: 1823


🏁 Script executed:

rg -n "is_done" include/boost/http/serializer.hpp -B 2 -A 3

Repository: cppalliance/http

Length of output: 4443


🏁 Script executed:

rg -n "prepare.*arr\|prepare.*empty" src/server/ include/boost/http/server/ --type cpp -B 3 -A 5

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

sed -n '700,730p' include/boost/http/serializer.hpp

Repository: cppalliance/http

Length of output: 810


🏁 Script executed:

python3 << 'EOF'
# Analyze the serve_static pattern
code = """
while(remaining > 0)
{
    capy::mutable_buffer arr[1];
    auto bufs = rp.res_body.prepare(arr);
    if(bufs.empty())
    {
        auto [ec2] = co_await rp.res_body.commit(0);
        if(ec2)
            co_return route_error(ec2);
        continue;
    }
    
    auto const n1 = f.read(bufs[0].data(), to_read, ec);
    if(ec.failed() || n1 == 0)
        break;
    
    auto [ec2] = co_await rp.res_body.commit(n1);
    if(ec2)
        co_return route_error(ec2);
    remaining -= static_cast<std::int64_t>(n1);
}
"""

print("Control flow analysis:")
print("1. Outer loop condition: while(remaining > 0)")
print("2. 'remaining' is only decremented on line 324 (normal path)")
print("3. When bufs.empty() is true (line 305):")
print("   - commit(0) is awaited")
print("   - continue is called (no 'remaining' decrement)")
print("4. Question: Can remaining stay the same while bufs.empty() repeatedly?")
print("   - Yes, if commit(0) doesn't make buffer space available")
print("   - This would cause unbounded retries at await points")
EOF

Repository: cppalliance/http

Length of output: 494


🏁 Script executed:

rg -n "stream_prepare\(\)" src/ include/ --type cpp -B 5 -A 10 | head -100

Repository: cppalliance/http

Length of output: 4804


🏁 Script executed:

sed -n '588,615p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 603


🏁 Script executed:

rg -n "out_prepare\|out_capacity" src/serializer.cpp -B 3 -A 8 | head -80

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

grep -n "stall\|timeout\|retry" src/server/serve_static.cpp

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

sed -n '655,680p' include/boost/http/serializer.hpp

Repository: cppalliance/http

Length of output: 685


🏁 Script executed:

sed -n '540,580p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 979


🏁 Script executed:

rg -n "out_prepare\|out_commit" src/serializer.cpp -B 2 -A 10 | head -120

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

sed -n '280,320p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 974


🏁 Script executed:

sed -n '560,600p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 947


🏁 Script executed:

rg -n "out_prepare" src/serializer.cpp -B 5 -A 15

Repository: cppalliance/http

Length of output: 2454


🏁 Script executed:

sed -n '450,500p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 1806


🏁 Script executed:

sed -n '671,690p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 491


🏁 Script executed:

rg -n "capy::buffer_pair\|out_\|in_" src/serializer.cpp | head -40

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

python3 << 'EOF'
# Trace the flow when prepare() returns empty in serve_static

print("=== Scenario: prepare() returns empty ===")
print()
print("serve_static (lines 305-310):")
print("  auto bufs = rp.res_body.prepare(arr);  // Returns empty span")
print("  if(bufs.empty()) {")
print("    auto [ec2] = co_await rp.res_body.commit(0);")
print("    continue;")
print("  }")
print()
print("sink.prepare() (serializer.hpp:622-635):")
print("  auto bufs = sr_->stream_prepare();  // Returns mutable_buffer_pair")
print("  // Loop fills dest[0] if bufs[0].size() > 0, else breaks")
print("  return dest.first(0);  // Empty result if all bufs have size 0")
print()
print("sink.commit() (serializer.hpp:648-680):")
print("  sr_->stream_commit(0);  // Commits 0 bytes")
print("  while(!sr_->is_done()) {")
print("    auto cbs = sr_->prepare();  // Calls sr_->prepare()")
print("    if(capy::buffer_empty(*cbs)) {")
print("      sr_->consume(0);  // Advances state machine")
print("      continue;")
print("    }")
print("    auto [ec, written] = co_await stream_->write_some(*cbs);")
print("    sr_->consume(written);")
print("    if(ec) co_return {ec};")
print("  }")
print("  co_return {};  // Returns successfully")
print()
print("Key question:")
print("  If stream_prepare() initially returned empty buffers,")
print("  how does is_done() become true?")
print("  - If is_done() is already true, the while loop never executes")
print("  - If is_done() is false, loop executes sr_->prepare()")
print("      which may still return empty buffers")
print("      causing consume(0) and continue indefinitely")
print()
print("  The critical path: Does sr_->prepare()'s behavior change")
print("  after sr_->consume(0) is called?")
EOF

Repository: cppalliance/http

Length of output: 1368


🏁 Script executed:

sed -n '671,700p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 789


🏁 Script executed:

sed -n '684,720p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 908


🏁 Script executed:

sed -n '200,250p' src/serializer.cpp

Repository: cppalliance/http

Length of output: 970


🏁 Script executed:

rg -n "is_done\|state_" src/serializer.cpp | head -50

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

python3 << 'EOF'
# Analyze the actual behavior when prepare() returns empty

print("=== Control flow when prepare(arr) returns empty ===\n")

print("Scenario: serve_static calls prepare(arr) and gets empty result")
print()
print("1. sink.prepare(arr) at line 304:")
print("   - calls sr_->stream_prepare()")
print("   - if stream_prepare() returns empty buffers (because out_.capacity() == 0)")
print("   - prepare(arr) returns dest.first(0) → empty span")
print()
print("2. sink.commit(0) at line 307:")
print("   - calls sr_->stream_commit(0)")
print("   - for chunked: if(n == 0) return; → early return, no state change!")
print("   - then enters while(!sr_->is_done()) loop")
print("   - calls sr_->prepare() [the serializer's prepare, not stream_prepare]")
print("   - this tries to flush internal serialization state")
print("   - calls write_some() to send data to network")
print("   - eventually exits when is_done() is true")
print()
print("3. Back in serve_static, next iteration:")
print("   - calls prepare(arr) again")
print("   - but stream_prepare() might STILL return empty")
print("   - because write_some() in the background may not have completed yet")
print()
print("KEY ISSUE:")
print("- serve_static loop is unbounded")
print("- No retry counter, no timeout")
print("- Relies entirely on write_some() making progress")
print("- If the network buffer fills and write_some() stalls...")
print("  the entire coroutine hangs without timeout")
EOF

Repository: cppalliance/http

Length of output: 1198


🏁 Script executed:

rg -n "while.*remaining\|while.*!.*done" src/server/serve_static.cpp -B 2 -A 8

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

sed -n '1,50p' src/server/serve_static.cpp

Repository: cppalliance/http

Length of output: 1271


🏁 Script executed:

cat test/unit/server/serve_static.cpp | head -100

Repository: cppalliance/http

Length of output: 435


🏁 Script executed:

rg -n "prepare.*empty\|commit.*0" test/unit/server/serve_static.cpp -B 3 -A 5 | head -80

Repository: cppalliance/http

Length of output: 42


🏁 Script executed:

python3 << 'EOF'
# Final analysis: is the review comment valid?

print("=== Final Analysis ===\n")
print("CLAIM: Unbounded retry when prepare() returns empty")
print()
print("Evidence found:")
print("1. serve_static loop condition: while(remaining > 0)")
print("   - No iteration cap or timeout")
print()
print("2. When prepare() returns empty (line 305):")
print("   - Calls co_await commit(0)")
print("   - Calls continue (no remaining decrement)")
print()
print("3. Inside commit() (serializer.hpp:648-680):")
print("   - Calls sr_->stream_commit(0)")
print("   - Enters while(!sr_->is_done()) draining loop")
print("   - Calls write_some() to flush buffers to network")
print("   - Exits when is_done() true")
print()
print("4. Key mechanism for forward progress:")
print("   - commit(0) is co_await, so it yields control")
print("   - The draining loop flushes internal buffers via write_some()")
print("   - write_some() is async and processes network events")
print("   - Once space is freed, stream_prepare() should return non-empty")
print()
print("VERDICT on original review:")
print("- The concern about unbounded retries is PARTIALLY VALID")
print("- However, the code relies on async progress via write_some()")
print("- The loop is NOT truly 'unbounded' in a blocking sense")
print("- It yields control at await points, allowing network progress")
print()
print("BUT: The original review's core concern remains valid:")
print("- No explicit safeguard (timeout, iteration limit)")
print("- Relies on implicit assumption that write_some() makes progress")
print("- If network stalls, the loop could hang indefinitely")
print("- This is a valid design concern worth addressing")
EOF

Repository: cppalliance/http

Length of output: 1389


Clarify forward progress mechanism for prepare() when returning empty buffers.

When rp.res_body.prepare(arr) returns empty (internal buffer full), the code calls co_await commit(0) and retries. This loop has no explicit iteration cap or timeout. While commit(0) yields control via co_await (allowing async network operations to drain the buffer), forward progress depends entirely on the internal write_some() flushing data successfully. Explicitly document or verify that capy's async machinery guarantees buffer space becomes available after a successful commit(0) call. If that guarantee cannot be relied upon, add an iteration limit or explicit timeout.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/server/serve_static.cpp` around lines 303 - 311, The loop that calls
rp.res_body.prepare(arr) and, on empty result, awaits rp.res_body.commit(0)
lacks a forward-progress cap; update serve_static.cpp to either document the
guarantee from capy that a successful co_await rp.res_body.commit(0) will make
space (i.e., commit triggers underlying write_some and frees buffer) or
implement a bounded retry/timeout: add a max retry counter (e.g.,
MAX_PREPARE_RETRIES) around the prepare/commit loop, after which return
route_error or a timeout error, and optionally insert a short async
backoff/yield between commits; ensure you reference and modify the loop that
uses capy::mutable_buffer arr[], rp.res_body.prepare(...), and
rp.res_body.commit(0) and return route_error(ec) when the retry limit or timeout
is reached.


auto const to_read = static_cast<std::size_t>(
(std::min)(remaining, static_cast<std::int64_t>(buf_size)));
(std::min)(remaining,
static_cast<std::int64_t>(bufs[0].size())));

auto const n1 = f.read(buffer, to_read, ec);
if(ec.failed() || n1 == 0)
auto const n1 = f.read(bufs[0].data(), to_read, ec);
if(ec.failed())
co_return route_error(ec);
if(n1 == 0)
break;

auto [ec2, n2] = co_await rp.res_body.write(
capy::const_buffer(buffer, n1));
(void)n2;
auto [ec2] = co_await rp.res_body.commit(n1);
if(ec2)
co_return route_error(ec2);
remaining -= static_cast<std::int64_t>(n1);
}

auto [ec3] = co_await rp.res_body.write_eof();
auto [ec3] = co_await rp.res_body.commit_eof(0);
if(ec3)
co_return route_error(ec3);
co_return route_done;
Expand Down
1 change: 1 addition & 0 deletions test/unit/Jamfile
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ project
<include>../../../url/extra/test_suite
<warnings>extra
<warnings-as-errors>on
<toolset>gcc:<cxxflags>"-Wno-maybe-uninitialized" # https://github.com/boostorg/url/issues/979
<link>static
;

Expand Down
7 changes: 3 additions & 4 deletions test/unit/bcrypt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include <boost/http/bcrypt.hpp>

#include <boost/capy/coro.hpp>
#include <boost/capy/ex/execution_context.hpp>
#include <boost/capy/ex/run_async.hpp>
#include <boost/capy/ex/system_context.hpp>
Expand Down Expand Up @@ -51,12 +50,12 @@ struct test_executor
void on_work_started() const noexcept {}
void on_work_finished() const noexcept {}

void dispatch(capy::coro h) const
std::coroutine_handle<> dispatch(std::coroutine_handle<void> h) const
{
h.resume();
return h;
}

void post(capy::coro h) const
void post(std::coroutine_handle<void> h) const
{
h.resume();
}
Expand Down
7 changes: 3 additions & 4 deletions test/unit/json/json_sink.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include <boost/capy/buffers.hpp>
#include <boost/capy/buffers/make_buffer.hpp>
#include <boost/capy/concept/write_sink.hpp>
#include <boost/capy/coro.hpp>
#include <boost/capy/ex/execution_context.hpp>
#include <boost/capy/ex/run_async.hpp>
#include <boost/capy/io_result.hpp>
Expand Down Expand Up @@ -68,14 +67,14 @@ struct test_executor
void on_work_started() const noexcept {}
void on_work_finished() const noexcept {}

void dispatch(capy::coro h) const
std::coroutine_handle<> dispatch(std::coroutine_handle<void> h) const
{
if(dispatch_count_)
++(*dispatch_count_);
h.resume();
return h;
}

void post(capy::coro h) const
void post(std::coroutine_handle<void> h) const
{
h.resume();
}
Expand Down
Loading