os400: rewrite upper ebcdic wrappers using dynbuf.

[monnerat]

This is a reopening of #21129

[Copilot]

Pull request overview

Reworks OS/400 CCSID conversion wrappers around dynbuf and libcurl allocators.

Changes:

• Adds iterative iconv() conversion through dyn_addn_CCSID().
• Migrates wrappers to dynamic buffers and libcurl memory allocation.
• Updates CCSID handling across forms, MIME, options, URLs, and headers.

Suppressed comments (5)

projects/OS400/ccsidcurl.c:339

• Reject negative lengths before passing this value to dyn_addn_CCSID. Except for -1, a negative int is converted to a huge finite size_t, causing iconv() to read beyond string; the underlying curl_easy_escape() contract rejects negative lengths.

  if(!string)
    return NULL;

projects/OS400/ccsidcurl.c:1385

• This introduces an approximately 8 MB ceiling for MIME memory payloads: dyn_addn_CCSID() returns CURLE_TOO_LARGE once the converted bytes plus its temporary terminator exceed this bound. curl_mime_data() accepts arbitrary size_t payload lengths, and the previous implementation sized the conversion buffer from datasize, so larger valid MIME parts now fail. Derive the dynbuf bound from the finite datasize with checked arithmetic (while handling CURL_ZERO_TERMINATED separately).

  curlx_dyn_init(&db, CURL_MAX_INPUT_LENGTH);

  result = dyn_addn_CCSID(&db, data, datasize, ccsid, ASCII_CCSID);

projects/OS400/ccsidcurl.c:1071

• The fixed CURL_MAX_INPUT_LENGTH bound now applies to both explicitly sized CURLOPT_COPYPOSTFIELDS data and blob options using this shared buffer. Those APIs can carry payloads larger than 8 MB, and the old implementation allocated from the supplied payload length; such calls now fail with CURLE_TOO_LARGE. Size the conversion buffer from the payload length in the relevant switch branch, with checked expansion and terminator space.

  curlx_dyn_init(&db, CURL_MAX_INPUT_LENGTH);

projects/OS400/ccsidcurl.c:375

• This path needs finite validated lengths at both conversion stages. A negative length below -1 becomes a huge size_t here and can make iconv() read past string; after unescaping, %00 can produce embedded NUL bytes that are truncated by the later CURL_ZERO_TERMINATED conversion. Reject negative input lengths, capture the decoded byte count locally, and convert exactly that many bytes.

  if(dyn_addn_CCSID(&db, string, length ? length : CURL_ZERO_TERMINATED,
                    ccsidin, ASCII_CCSID))
    return NULL;

  d = curl_easy_unescape(handle,

projects/OS400/ccsidcurl.c:735

• This hard-caps explicitly sized legacy form data conversion at roughly 8 MB, although form content lengths are payload sizes and the previous code allocated from that supplied length. Large valid CURLFORM_CONTENTSLENGTH/CURLFORM_CONTENTLEN values therefore regress to a conversion failure. For finite lengths, initialize the dynbuf from the checked input length rather than the generic string-input limit.

  curlx_dyn_init(&db, CURL_MAX_INPUT_LENGTH);

  if(lengthx >= 0)
    len = (size_t) forms[lengthx].value;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[monnerat]

We should probably add this to the code style document. We didn't before because it was never an issue.

The issue is more global: continuous never-ending housekeeping interferes with code development/fix. There should be periods for this, a bit like we have "feature freeze". Just a suggestion trying to make everyone happy.

Looking at existing source code, typecasts are written without an adjacent space to the right of the closing paren.

Yes: this is so nowadays because it has been silently bulk-changed everywhere.

Even while it isn't written in the current code style document, I think it makes sense to follow the same style as the rest of the curl code.

If you think it is part of the style, please write it down, and add it to checksrc.pl if possible.
There is already a lot of lexical rules that are not my natural way of coding and I often miss them while coding. Fortunately, checksrc.pl helps me fix them before PR. Examples:

• Use of TAB,
• No space between a reserved word and the left parent,
• ? : surrounded by spaces,
• sizeof argument with mandatory parents,
• ... and more

As I wrote before, I'm not against rules (providing they are not too constraining for the developer), but they have to be explicit and accessible as a reference. New ones should be advertised.

The target swap occurred because I started edition before the bulk-change that suppressed the space: it was then lost in hundreds of conflicts.

The program compiles, runs and is readable anyway, which is IMHO the main goal of a project. Such make-time undetected details (without minimizing the janitor's work) should not be subject to moaning. just to be adapted if judicious.

When something breaks on the OS400 because other contributor can't try compiling it, I just fix it afterwards when detected (i.e.: see 64c03bb) because this falls to me, although other developers kindly try to improve OS400 code blindly. Likewise, I try to do my best on others' targets, but you have to admit by analogy it is hard to respect something unspecified and unchecked you're not specialized on and escapes a minded logic. It is also hard to determine which latitude has a developer for creation if rules are not explicit.

[bagder]

If you think it is part of the style, please write it down

Done.

and add it to checksrc.pl if possible.

I have not figured out how to do that. It requires quite complicated parsing and analysis beyond what the script is currently doing.

[monnerat]

Done.

Thanks. On my side, all casts in this PR have been changed.

I have not figured out how to do that. It requires quite complicated parsing and analysis beyond what the script is currently doing.

What about something like
/\(.*?\)\s+[A-Za-z0-9_(+*-]/
? (probably to be fine tuned).
It won't detect casts with nested parentheses, but they do not occur frequently.
Since calls and assignments may not start on the same line as the enclosing compound statement, they should not be detected as false positives.

May I suggest such rule change/addition be advertised on the mailing list ?

[monnerat]

Thanks for merge.