Skip to content

Validate argv entries before std::string construction in ParseCLI#173

Closed
jmestwa-coder wants to merge 1 commit into
Taywee:masterfrom
jmestwa-coder:parsecli-null-argv-validation
Closed

Validate argv entries before std::string construction in ParseCLI#173
jmestwa-coder wants to merge 1 commit into
Taywee:masterfrom
jmestwa-coder:parsecli-null-argv-validation

Conversation

@jmestwa-coder
Copy link
Copy Markdown
Contributor

Summary

ParseCLI(argc, argv) previously bulk-constructed std::string objects directly from the supplied argv range.

If an interior argv entry is nullptr, this results in construction of std::string(nullptr), which is undefined behavior and can crash depending on the standard library implementation.

This change validates each argument pointer before string construction and converts malformed input containing null entries into a deterministic parse failure.

Changes

  • Replaced bulk args.assign(argv + 1, argv + argc) construction with per-entry validation
  • Rejects nullptr entries before std::string construction
  • Returns Error::Parse in ARGS_NOEXCEPT builds
  • Throws ParseError in normal builds
  • Preserves behavior for well-formed argv inputs

Tests

Added regression coverage for:

  • malformed argv containing interior null entries
  • throwing mode behavior
  • ARGS_NOEXCEPT behavior

@Taywee
Copy link
Copy Markdown
Owner

Taywee commented May 24, 2026

In what case would argv[i] == nullptr where i < argc? The PR looks fine, but I'm not sure it's protecting against anything that would ever actually happen, unless somebody is constructing an argc and argv that are badly formed, in which case, I don't think that's our responsibility to defend against. If the caller builds an array with inner null pointers or dangling pointers, that's on them, not us. This feels over-defensive.

@Taywee
Copy link
Copy Markdown
Owner

Taywee commented May 24, 2026

If anything at all, this should be some kind of debug assert, but I don't think even that. argc and argv conventions are well known, and there are so many ways that the understanding can be abused. Bad argc value, null pointers, dangling pointers, pointers to non-char data. We shouldn't protect against error conditions that literally no program will ever hit.

@jmestwa-coder
Copy link
Copy Markdown
Contributor Author

@Taywee.
Understood, thanks for the clarification.

My intent here was specifically to avoid the std::string(nullptr) UB path for malformed caller-constructed argv arrays, but I understand and respect the project's position that these states are considered caller responsibility rather than something the library should validate internally.

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.

2 participants