Skip to content

fix: grow NgxString on write! instead of failing within capacity - #333

Merged
bavshin-f5 merged 1 commit into
nginx:mainfrom
u5surf:fix/ngxstring-write-grows
Sep 17, 2026
Merged

bavshin-f5 merged 1 commit into
nginx:mainfrom
u5surf:fix/ngxstring-write-grows

Conversation

@u5surf

@u5surf u5surf commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

Closes #326, taking the fix you preferred there.

 fn write_str(&mut self, s: &str) -> fmt::Result {
-    self.append_within_capacity(s).map_err(|_| fmt::Error)
+    self.try_append(s).map_err(|_| fmt::Error)
 }

try_append reserves before it appends, so allocation stays checked: a genuine
failure surfaces as fmt::Error rather than a panic, which is the property
String does not give. It also means a failed write leaves the string
untouched instead of holding the part that fit.

No separate fixed-size formatter. append_within_capacity is unchanged and
still public for anyone who wants that behaviour directly.

Testing

Two tests added:

  • write! into a string with no capacity reserved now produces the whole
    value, which previously failed on the first byte.
  • write! backed by an allocator that always refuses returns an error, does
    not panic, and leaves the string empty.

The existing test_string_write is untouched and still passes. It reserves up
front and asserts the buffer pointer and capacity do not change, which holds
because try_reserve_exact does nothing when the spare capacity already covers
the write. So the reserve-once-then-fill pattern in examples/shared_dict.rs
keeps its single allocation.

Checklist

  • I have written my commit messages in the Conventional Commits format.
  • I have read the CONTRIBUTING doc
  • I have added tests (when possible) that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

fmt::Write was wired to append_within_capacity, so a write! into a
freshly created NgxString failed on the first byte, and one into a
partly filled one wrote what fit and returned an error alongside the
truncated value.

Use try_append, which reserves before it appends. Allocation stays
checked, so a genuine failure still surfaces as fmt::Error rather than a
panic, and a failed write leaves the string untouched rather than half
written.

Reserving up front is unaffected: try_reserve_exact does nothing when the
spare capacity already covers the write, so the existing test asserting
that no reallocation occurs still holds.

Closes nginx#326

Signed-off-by: Y.Horie <u5.horie@gmail.com>

@bavshin-f5 bavshin-f5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks!

@bavshin-f5
bavshin-f5 merged commit f5aacab into nginx:main Sep 17, 2026
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.

fmt::Write for NgxString does not grow the string, and leaves a partial write on overflow

2 participants