Fix ArgumentOutOfRangeException when header value is empty

[benyuz]

Description

• Added bounds check before Substring call in WebHeaderCollection.Add(string header) method.
• Prevents ArgumentOutOfRangeException when header ends with ':' and has no value (e.g., "Authorization:").

Motivation and Context

• Currently, calling Add(string header) with a header string that has no value after the colon (e.g., "Authorization:") causes a crash because Substring(colpos + 1) is called without checking bounds.
• This issue was discovered during development of an embedded Web API server (PicoServer.Nano) running on ESP32.

How Has This Been Tested?

• Manually tested on ESP32 device running nanoFramework using curl -H "Authorization:" http://<device-ip>/hello.
• Verified that the exception no longer occurs and the request is handled gracefully.

Screenshots

Types of changes

• Bug fix (non-breaking change which fixes an issue with code or algorithm)

Checklist:

• My code follows the code style of this project.
• My changes require an update to the documentation.
• I have read the CONTRIBUTING document.
• I have tested everything locally and all new and existing tests passed.
• I have added new tests to cover my changes.

[coderabbitai]

📝 Walkthrough

Walkthrough

WebHeaderCollection.Add(string header) now handles a trailing colon by assigning an empty string when the colon is the last character instead of calling Substring. A new test class WebHeaderCollectionTests was added with unit tests covering Authorization header parsing and various invalid/edge-case header inputs.

Changes

HTTP Header Bounds Check

Layer / File(s)	Summary
Parsing Fix nanoFramework.System.Net.Http/Http/System.Net.WebHeaders.cs	In WebHeaderCollection.Add(string header), replace unconditional header.Substring(colpos + 1) with: if colpos + 1 >= header.Length then value = string.Empty; otherwise value = header.Substring(colpos + 1).
Tests / Coverage Tests/HttpUnitTests/WebHeaderCollectionTests.cs	Add WebHeaderCollectionTests with multiple [TestMethod]s covering Authorization header parsing (valid forms and edge cases that yield string.Empty) and negative tests asserting exceptions for null, empty, missing-colon, and header-name-with-space inputs.
Project Wiring Tests/HttpUnitTests/HttpUnitTests.nfproj	Include the new test source file by adding <Compile Include="WebHeaderCollectionTests.cs" /> to the project file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Type: Unit Tests, Area: Config-and-Build

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is directly related to the changeset, explaining the bug, its cause, motivation, and testing performed.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and accurately describes the main fix: preventing ArgumentOutOfRangeException when a header value is empty (colon at end of header string). It is concise (60 characters, slightly over the 50-character ideal but acceptably brief) and directly relates to the primary changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[benyuz]

 已收到您的邮件,我会及时查看的!

[benyuz]

@dotnet-policy-service agree

[Copilot]

Pull request overview

Fixes a crash in the nanoFramework System.Net header parsing path by safely handling header strings that end with a colon and have no value (e.g., "Authorization:") when using WebHeaderCollection.Add(string header).

Changes:

• Adds a bounds check before extracting the header value substring in WebHeaderCollection.Add(string header).

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@nanoFramework.System.Net.Http/Http/System.Net.WebHeaders.cs`:
- Around line 430-439: Replace the multi-line if/else that assigns value based
on colpos and header with a concise ternary assignment: set value = (colpos + 1
>= header.Length) ? string.Empty : header.Substring(colpos + 1); locating the
logic that uses the variables header, colpos and the local variable value inside
the WebHeaders parsing code (the block currently handling "Handle empty header
value") and swap the block for the single-line ternary form.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b3f50d60-6a7b-46ee-bcc6-462dc3c49dd0

📥 Commits

Reviewing files that changed from the base of the PR and between 2aa56ad and 85ec85e.

📒 Files selected for processing (1)

• nanoFramework.System.Net.Http/Http/System.Net.WebHeaders.cs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Tests/HttpUnitTests/WebHeaderCollectionTests.cs`:
- Around line 14-26: Add a regression test that covers the exact crash case
where the header name is present but there is no value after the colon; in the
WebHeaderCollectionTests class add a new test method (e.g.,
Add_Authorization_EmptyValue_ShouldNotThrow) which creates a WebHeaderCollection
and calls headers.Add("Authorization:") and asserts that no exception is thrown
(mirror the pattern used in
Add_Authorization_BearerWithSpaceAndNoValue_ShouldNotThrow and
Add_Authorization_NoSpaceSingleChar_ShouldNotThrow) so the code path in
WebHeaderCollection.Add that handles colpos + 1 >= header.Length is exercised.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 75f60ac4-d697-452f-a0c4-0fda7a5221a5

📥 Commits

Reviewing files that changed from the base of the PR and between 85ec85e and f6dcce0.

📒 Files selected for processing (1)

• Tests/HttpUnitTests/WebHeaderCollectionTests.cs

[josesimoes]

LGTM!
I've added more unit tests that are meant to throw.
I've also added this new unit test group to the project as it wasn't building nor being executed.

[josesimoes]

/azp runpipeline

[azure-pipelines]

Command 'runpipeline' is not supported by Azure Pipelines.

Supported commands

• help:
• Get descriptions, examples and documentation about supported commands
• Example: help "command_name"
• list:
• List all pipelines for this repository using a comment.
• Example: "list"
• run:
• Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
• Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
• where:
• Report back the Azure DevOps orgs that are related to this repository and org
• Example: "where"

See additional documentation.

[josesimoes]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Tests/HttpUnitTests/WebHeaderCollectionTests.cs`:
- Around line 73-99: The tests in WebHeaderCollectionTests (methods
Add_NullHeader_ThrowsArgumentNullException,
Add_EmptyHeader_ThrowsArgumentNullException,
Add_HeaderWithNoColon_ThrowsArgumentException,
Add_HeaderNameWithSpace_ThrowsArgumentException) use Assert.ThrowsException
which does not exist in the current test framework; replace each
Assert.ThrowsException(...) call with Assert.Throws(...) using the same
exception type and lambda/action argument so the tests compile and correctly
assert exceptions (follow the pattern used in HttpClientTest.cs).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6f380f96-dbdf-4a03-9ca7-3d56b39922e0

📥 Commits

Reviewing files that changed from the base of the PR and between f6dcce0 and f584fd9.

📒 Files selected for processing (2)

• Tests/HttpUnitTests/HttpUnitTests.nfproj
• Tests/HttpUnitTests/WebHeaderCollectionTests.cs

[nfbot]

@benyuz thank you again for your contribution! 🙏😄

.NET nanoFramework is all about community involvement, and no contribution is too small.
We would like to invite you to join the project's Contributors list.

Please edit it and add an entry with your GitHub username in the appropriate location (names are sorted alphabetically):

  <tr>
    <td><img src="https://github.com/benyuz.png?size=50" height="50" width="50" ></td>
    <td><a href="https://github.com/benyuz"></a></td>
  </tr>

(Feel free to adjust your name if it's not correct)