Skip to content

[AtModem] Fix ArgumentOutOfRangeException when header value is empty#1538

Merged
josesimoes merged 1 commit into
nanoframework:mainfrom
benyuz:main
May 14, 2026
Merged

[AtModem] Fix ArgumentOutOfRangeException when header value is empty#1538
josesimoes merged 1 commit into
nanoframework:mainfrom
benyuz:main

Conversation

@benyuz
Copy link
Copy Markdown
Contributor

@benyuz benyuz commented May 4, 2026

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.
  • In sync with Fix ArgumentOutOfRangeException when header value is empty System.Net.Http#488.

How Has This Been Tested?

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 updated the documentation accordingly.
  • 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.

@nfbot nfbot added the Type: bug Something isn't working label May 4, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0dd7eabe-f993-4769-bfb1-5549701e90cd

📥 Commits

Reviewing files that changed from the base of the PR and between 853bee1 and 1d9b638.

📒 Files selected for processing (1)
  • devices/AtModem/Http/Http/WebHeaderCollection.cs

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Fixed handling of malformed header strings to prevent application crashes.

Walkthrough

The WebHeaderCollection.Add(string header) method now checks bounds before extracting the value after :; if the colon is the last character, the value is set to string.Empty instead of calling Substring and throwing an exception.

Changes

HTTP Header Bounds Validation

Layer / File(s) Summary
Core Logic
devices/AtModem/Http/Http/WebHeaderCollection.cs
Add(string header) adds bounds check: if colpos + 1 >= header.Length, set value to string.Empty; otherwise extract substring from colpos + 1. Prevents ArgumentOutOfRangeException on headers with trailing colons.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main fix: preventing ArgumentOutOfRangeException when header values are empty, which directly matches the changeset.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, covering the issue, motivation, testing approach, and alignment with related work.
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.

✏️ 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

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

@benyuz
Copy link
Copy Markdown
Contributor Author

benyuz commented May 4, 2026

@dotnet-policy-service agree

@josesimoes
Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@josesimoes
Copy link
Copy Markdown
Member

/azp runpipeline

@azure-pipelines
Copy link
Copy Markdown

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
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@josesimoes josesimoes changed the title Fix ArgumentOutOfRangeException when header value is empty [AtModem] Fix ArgumentOutOfRangeException when header value is empty May 4, 2026
Fix bounds check when header value is empty (e.g., "Authorization:")
@josesimoes
Copy link
Copy Markdown
Member

/azp run

@josesimoes josesimoes enabled auto-merge (squash) May 14, 2026 07:05
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@josesimoes josesimoes merged commit 20bca36 into nanoframework:main May 14, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants