Fix: in the Web add-on, packets may be incorrectly released in case of errors

[Yves57]

data_packet_ptr is released while it is never initialized at this point.

[fdesbiens]

Thank you for this contribution, @Yves57. Please excuse the delay in getting back to you as I was on vacation.

Before we can accept this PR, you need to sign the Eclipse Contributor Agreement (ECA). The purpose of the ECA is to provide a written record that you have agreed to provide your code and documentation contributions under the licenses used by the Eclipse ThreadX project. It also makes it clear that you are promising that what you are contributing to Eclipse is code you wrote, and you have the necessary rights to contribute it to our projects. And finally, it documents a commitment from you that your open source contributions will be permanently on the public record.

Signing the ECA requires an Eclipse Foundation account if you do not already have one. You can create one for free at https://accounts.eclipse.org.

Be sure to use the same email address when you register for the account that you intend to use on Git commit records. Also, please add your GitHub ID to your Eclipse account. This enables synchronisation between Eclipse-owned infrastructure and GitHub.

Here is the link to sign the ECA:
https://accounts.eclipse.org/user/login?destination=user/eca

[Yves57]

@fdesbiens It should be good for the agreement.

[rahmanih]

Hi @Yves57
Could you please set the dev as merge branch instead of master
regrards

[fdesbiens]

Thanks Yves — good catch, and the diagnosis was exactly right. data_packet_ptr is declared without an initializer at nx_web_http_server.c:3919 and is not assigned until _nx_web_http_server_packet_get() runs at :4293, so both of the releases you removed were operating on whatever the stack slot happened to hold. Removing them is the correct fix rather than moving or guarding them: the equivalent underflow branches in the legacy HTTP add-on (addons/http/nxd_http_server.c:3867-3880 and :3913-3926) contain no release either, so your change brings the Web add-on back in line with that implementation.

I have pushed two follow-up commits to your branch and retargeted the PR from master to dev. Here is what changed and why.

1. Release the request packet on the chunked path

b1815cf — Added chunked packet release to the PUT underflow error paths

After your change, the two underflow exits became the only error exits in _nx_web_http_server_put_process() that release nothing at all. Every neighbouring exit releases the request packet when the request is chunked — the file-write failures at :4191-4194 and :4244-4248, and the normal path at :4283-4286.

That is harmless today, because a chunked request cannot actually reach either underflow branch: length is initialised from packet_ptr -> nx_packet_length at :4117 with offset = 0, so consumed_length can never exceed it and the subtraction cannot wrap. But that is a subtle invariant to depend on for correctness, and it would break quietly if the chunked length bookkeeping ever changed. The two exits now use the same guard as their neighbours, so all the error paths in the function are uniform.

2. A regression test for the underflow path

a5c4d07 — Added a regression test for the PUT Content-Length underflow path

New file: test/regression/web_test/netx_web_put_content_length_underflow_test.c, registered in test/cmake/web/regression/CMakeLists.txt and test/regression/web_test/netx_https_testcontrol.c.

The underflow detection had no coverage at all — netx_web_put_basic_test.c drives the nx_web_http_client_put_* API, which would never emit a Content-Length that disagrees with the body it sends, so nothing in the suite reached :4201 or :4255. The new test uses a raw TCP client to send a PUT announcing five content bytes while carrying ten, all in one packet, and checks that the server answers 400 and that its packet pool is fully restored afterwards.

One thing worth being explicit about, since it is easy to assume otherwise: this test does not fail on the unpatched code. What the removed calls did depended entirely on the contents of an uninitialized stack slot, which is not something a test can pin down. I verified this by reintroducing the two calls and re-running — the test still passed. So please read it as coverage that locks in the correct behaviour of a previously untested branch, not as a reproducer for the defect you fixed. The pool-availability assertion is there to catch leaks and double releases on this path in future.

Verification

Built and run with CMake and Ninja across all three web configurations — default_build_coverage, no_tls_build_coverage and digest_authenticate_build. 54/54 tests pass in each, and the build is clean with no new warnings under -Werror -Wall -Wextra.

One process note

Contributions to this project should be based on dev rather than master; master is updated by promotion from dev. I have already changed the base branch on the PR and the two branches were identical for this file, so there was nothing to resolve — no action needed from you, just something to keep in mind next time.

Thanks again for the fix and for the clear problem statement in the PR description. It made this an easy one to confirm.

If everything looks right to you, then I will merge this to dev right away. This will ship with our Q3 2026 release in September.

[Yves57]

@fdesbiens The additional modifications seem correct to me.