Skip to content

Conversation

@bagder
Copy link
Member

@bagder bagder commented Dec 22, 2025

Build list with dynbuf.

Build list with dynbuf.
@bagder bagder added the tidy-up label Dec 22, 2025
@bagder bagder requested a review from Copilot December 22, 2025 10:40
@bagder
Copy link
Member Author

bagder commented Dec 22, 2025

augment review

@bagder bagder marked this pull request as ready for review December 22, 2025 10:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the content encoding list building to use dynamic buffers (dynbuf) instead of fixed-size buffers and strcpy operations. The changes improve memory safety by avoiding potential buffer overflows.

Key changes:

  • Replaced Curl_all_content_encodings() with Curl_get_content_encodings() that returns a dynamically allocated string
  • Eliminated fixed-size 256-byte stack buffer in favor of dynamic allocation
  • Simplified error message to remove redundant encoding list generation

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
lib/content_encoding.h Updated function signature to return CURLcode and use output parameter for string
lib/content_encoding.c Refactored to build encoding list with dynbuf; simplified error message
lib/setopt.c Updated caller to use new API with proper error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@augmentcode
Copy link

augmentcode bot commented Dec 22, 2025

🤖 Augment PR Summary

Summary: Refactors content-encoding name list generation to avoid fixed-size buffers/strcpy.

Changes:

  • Adds `curlx/dynbuf.h` and builds the encoding list using `dynbuf`.
  • Replaces `Curl_all_content_encodings(buf, blen)` with `Curl_get_content_encodings()` returning an allocated string.
  • Updates `CURLOPT_ACCEPT_ENCODING` empty-string handling to use the new helper.
  • Simplifies the error path for unknown content encoding messages.

Technical Notes: Callers now need to handle ownership of the returned string and OOM behavior.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

@bagder bagder closed this in 6b9c75e Dec 22, 2025
@bagder bagder deleted the bagder/content-avoid-strcpy branch December 22, 2025 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

1 participant