Handle resume responses without Content-Range#27529
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates PowerShell web cmdlets’ -Resume handling to tolerate non-compliant 416 RequestedRangeNotSatisfiable responses that omit Content-Range, treating them as “already complete”, and adds test coverage for this behavior.
Changes:
- Add a WebListener endpoint that returns
416without aContent-Rangeheader when aRangerequest is made. - Refactor and harden resume-range handling in
WebRequestPSCmdletto avoid null dereferences and treat missingContent-Rangeas complete. - Add Pester tests for
Invoke-WebRequestandInvoke-RestMethodvalidating the new-Resumebehavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/tools/WebListener/Controllers/ResumeController.cs | Adds a new test endpoint to simulate 416 without Content-Range for range requests |
| test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 | Adds regression tests for -Resume behavior when Content-Range is missing |
| src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs | Refactors resume logic into helpers and treats missing Content-Range as “already complete” |
| string rangeHeader; | ||
| if (TryGetRangeHeader(out rangeHeader)) | ||
| { | ||
| Response.StatusCode = StatusCodes.Status416RequestedRangeNotSatisfiable; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Addressed in 4d9255e76: the unused rangeHeader local is now a discard in the missing Content-Range endpoint.
| await Response.Body.WriteAsync(FileBytes, 0, FileBytes.Length); | ||
| } | ||
|
|
||
| public async void MissingContentRange() |
There was a problem hiding this comment.
Addressed in 4d9255e76: MissingContentRange() now returns Task so exceptions flow through the request pipeline instead of using async void.
| // RFC 9110 only says 416 responses SHOULD include Content-Range. Treat a missing | ||
| // header as an already-complete resume instead of failing with a null reference. | ||
| return contentRange is null || (contentRange.HasLength && contentRange.Length == _resumeFileSize); |
There was a problem hiding this comment.
Addressed in 4d9255e76: the resume helper now reports whether Content-Range was missing, and the caller writes a distinct verbose message for that fallback instead of reusing the confirmed-size message.
|
Pushed
Local check: |
Problem
Invoke-WebRequest -ResumeandInvoke-RestMethod -Resumecan throw aNullReferenceExceptionwhen a server responds to an already-complete range request with416 Requested Range Not Satisfiablebut omits the optionalContent-Rangeheader.Fixes #23948.
Root Cause
The resume code dereferenced
response.Content.Headers.ContentRangefor every416response. RFC 9110 says a416response should includeContent-Range, but it is not mandatory, and some servers omit it when the requested range starts at EOF.Solution
416responses withoutContent-Rangeas an already-complete resumed download.Content-Rangeis present and shows the remote size differs from the local file size.416withoutContent-Range.Invoke-WebRequestandInvoke-RestMethod.Tests
NullReferenceExceptionlocally with a tiny HTTP server returning416withoutContent-Range.Start-PSBuild -Configuration Debug -NoPSModuleRestorePublish-PSTestToolsInvoke-WebRequest -ResumeandInvoke-RestMethod -Resumeagainst the newMissingContentRangeendpoint.git diff --check(only existing Windows line-ending notices)