nginx / nginx/ngx-rust

Pool::create_buffer cannot be filled through the safe API

Open
#327 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
915
Forks
96
Avg merge
5d 22h
Merged PRs (30d)
6

Description

Pool::create_buffer(n) allocates n bytes, but there is no safe way to put
anything into them:

let mut buf = request.pool().create_buffer(body.len()).unwrap();
buf.as_bytes_mut()[..body.len()].copy_from_slice(body); // panics

ngx_create_temp_buf leaves pos == last, so Buffer::len() is 0 and
MutableBuffer::as_bytes_mut() hands back an empty slice. That is consistent
with as_bytes_mut meaning "the contents", but it leaves the buffer
unfillable: neither trait exposes the capacity or a way to set the length, and
TemporaryBuffer has no inherent methods besides from_ngx_buf. The only
route is as_ngx_buf_mut() and advancing last by hand, which drops out of
the safe API.

create_buffer_from_str works and covers most response bodies, so this is not
urgent. It does mean create_buffer currently has no safe use, while the
obvious composition above compiles and then takes down the worker.

Something like TemporaryBuffer::append(&mut self, &[u8]) -> usize, or
exposing capacity next to a length setter, would close it.

I'd like to work on this issue.

Environment
  • ngx-rust at cda9d8372ccd2e139ef2b03e5806e019e7796187 (ngx 0.5.0)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at Pool::create_buffer and the TemporaryBuffer, Buffer::len, and MutableBuffer::as_bytes_mut APIs; compare them with create_buffer_from_str. Check how as_ngx_buf_mut currently permits advancing last, then add a safe way to fill the allocated buffer without an empty-slice panic. Done means create_buffer can be used safely for a byte body and the behavior is covered by a test.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend-api-design
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.