Skip to content

Handle content length being null in StreamClient #939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mskovacic wants to merge 1 commit into
base: master
Choose a base branch
from
+4 −5
Open

Handle content length being null in StreamClient #939

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions YoutubeExplode/Videos/Streams/StreamClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ private async IAsyncEnumerable<IStreamInfo> GetStreamInfosAsync(
}

var contentLength = await TryGetContentLengthAsync(streamData, url, cancellationToken);
if (contentLength is null)
continue;
var fileSize = contentLength is null ? new FileSize() : new FileSize(contentLength.Value);
Comment on lines 115 to +116
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

contentLength being null is now represented as new FileSize() (0 bytes). This is not equivalent to “unknown size” and will break downstream logic that relies on a real size (e.g., MediaStream.Length/segmenting uses streamInfo.Size.Bytes; a 0 length causes reads to terminate immediately and can yield empty downloads). Consider keeping size nullable (API change) or adding an explicit “unknown” representation and updating consumers (MediaStream, progress reporting) to handle unknown length without using 0 as a sentinel, or alternatively derive the length via a range request when Content-Length is absent.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

This change introduces new behavior (streams with unknown content length are now included in manifests), but there’s no corresponding coverage to ensure GetManifestAsync includes such streams and DownloadAsync/CopyToAsync still functions when the size cannot be determined (including progress reporting). Please add a test that simulates missing Content-Length and verifies the stream can still be downloaded correctly.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

PR description still contains the placeholder Closes #ISSUE_NUMBER. Please link the actual issue (or remove the close directive) so the change is properly tracked and auto-linked on merge.

Copilot uses AI. Check for mistakes.

var container =
streamData.Container?.Pipe(s => new Container(s))
Expand Down Expand Up @@ -151,7 +150,7 @@ streamData.VideoWidth is not null && streamData.VideoHeight is not null
var streamInfo = new MuxedStreamInfo(
url,
container,
new FileSize(contentLength.Value),
fileSize,
bitrate,
streamData.AudioCodec,
audioLanguage,
Expand All @@ -169,7 +168,7 @@ streamData.VideoWidth is not null && streamData.VideoHeight is not null
var streamInfo = new VideoOnlyStreamInfo(
url,
container,
new FileSize(contentLength.Value),
fileSize,
bitrate,
streamData.VideoCodec,
videoQuality,
Expand All @@ -185,7 +184,7 @@ streamData.VideoWidth is not null && streamData.VideoHeight is not null
var streamInfo = new AudioOnlyStreamInfo(
url,
container,
new FileSize(contentLength.Value),
fileSize,
bitrate,
streamData.AudioCodec,
audioLanguage,
Expand Down