Skip to content

feat: Add support for HTTP response trailers - #2417

Open
HosseinDinarvand wants to merge 1 commit into
restsharp:devfrom
HosseinDinarvand:fix/http-response-trailer
Open

HosseinDinarvand wants to merge 1 commit into
restsharp:devfrom
HosseinDinarvand:fix/http-response-trailer

Conversation

@HosseinDinarvand

Copy link
Copy Markdown

PR Classification

New feature and API enhancement for preserving HTTP trailer headers.

PR Summary

RestSharp now exposes HTTP response trailers separately from regular and content headers. Integration coverage verifies trailer preservation on .NET 8+.

  • RestResponse: Maps HttpResponseMessage.TrailingHeaders under NET targets.
  • RestResponseBase: Adds nullable TrailingHeaders with documentation.

Related to #2413

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Expose HTTP response trailers separately from other headers

✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Expose response trailers separately so callers can distinguish them from regular and content
 headers.
• Add .NET 8+ integration coverage for trailer preservation and header separation.
Diagram

graph TD
  H["Test handler"] --> M["HTTP response"] --> B["Body read"] --> R["RestResponse"] --> C["Caller"]
  M --> X["Header conversion"] --> R
Loading
High-Level Assessment

Keep the separate response property and reuse the existing header converter. Merging trailers into Headers would lose their distinction, while a new conversion path would duplicate existing behavior.

Files changed (3) +37 / -0

Enhancement (2) +8 / -0
RestResponse.csMap HTTP response trailers into RestResponse +3/-0

Map HTTP response trailers into RestResponse

• On NET targets, the default response mapping converts HttpResponseMessage.TrailingHeaders into HeaderParameter entries. Trailers remain separate from response and content headers.

src/RestSharp/Response/RestResponse.cs

RestResponseBase.csAdd a public TrailingHeaders response property +5/-0

Add a public TrailingHeaders response property

• Adds a documented, nullable read-only collection of HeaderParameter entries for response trailers. The property is available to RestResponse and its generic subclass.

src/RestSharp/Response/RestResponseBase.cs

Tests (1) +29 / -0
ResponseTrailerTests.csVerify trailer preservation and separation +29/-0

Verify trailer preservation and separation

• Adds a .NET 8+ integration test using a custom HTTP handler to return a Checksum trailer. It verifies that the trailer appears in TrailingHeaders but not in Headers or ContentHeaders.

test/RestSharp.Tests.Integrated/ResponseTrailerTests.cs

@sonarqubecloud

Copy link
Copy Markdown

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Older targets return null for trailers 🐞 Bug ≡ Correctness
Description
TrailingHeaders has no empty default, and its only assignment in GetDefaultResponse is excluded
from non-NET builds. Successful responses on .NET Framework and .NET Standard therefore leave the
property null, unlike modern targets, where responses without trailers expose an empty collection.
Code

src/RestSharp/Response/RestResponseBase.cs[117]

+    public IReadOnlyCollection<HeaderParameter>? TrailingHeaders { get; set; }
Evidence
The project targets netstandard2.0, net471, and net48 alongside modern .NET. The new property has no
initializer, and the only code that assigns it is guarded by #if NET, so successful responses on
the older targets retain its default null value.

src/Directory.Build.props[1-6]
src/RestSharp/Response/RestResponseBase.cs[114-117]
src/RestSharp/Response/RestResponse.cs[72-80]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Successful responses on older targets leave `TrailingHeaders` null because their builds exclude its only assignment.
## Fix Focus Areas
- src/RestSharp/Response/RestResponseBase.cs[114-117]
- src/RestSharp/Response/RestResponse.cs[76-78]
## Recommended Fix
Initialize `TrailingHeaders` to an empty collection for targets without trailer support, while preserving the populated collection on supported targets.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: Auto: ⚖️ Balanced: Public API and HTTP trailer lifecycle changes require careful review.

Grey Divider

Tip of the day
💡 Did you know, you can pick a display mode instead of tweaking every Display option by hand

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

/// <summary>
/// Trailer headers returned by server with the response
/// </summary>
public IReadOnlyCollection<HeaderParameter>? TrailingHeaders { get; set; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Older targets return null for trailers 🐞 Bug ≡ Correctness

TrailingHeaders has no empty default, and its only assignment in GetDefaultResponse is excluded
from non-NET builds. Successful responses on .NET Framework and .NET Standard therefore leave the
property null, unlike modern targets, where responses without trailers expose an empty collection.
Agent Prompt
## Issue description
Successful responses on older targets leave `TrailingHeaders` null because their builds exclude its only assignment.
## Fix Focus Areas
- src/RestSharp/Response/RestResponseBase.cs[114-117]
- src/RestSharp/Response/RestResponse.cs[76-78]
## Recommended Fix
Initialize `TrailingHeaders` to an empty collection for targets without trailer support, while preserving the populated collection on supported targets.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@HosseinDinarvand

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

This branch has not been deployed

No deployments
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.

1 participant