1753002 - Diagnostic assert at [@ mozilla::net::Http3Stream::ReadSegments ]

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Comment 1

[Kershaw Chang [:kershaw]]

Found this crash on try server.
https://treeherder.mozilla.org/logviewer?job_id=366855828&repo=autoland&lineNumber=6834

The test netwerk/test/unit/test_http3_large_post.js was running with socket process. Before running this test, there were already some test failures, which might suggest that something was already wrong.

Comment 2

[Kershaw Chang [:kershaw]]

Attachment: live_backing.log

The raw log for comment #1.

Comment 3

[Kershaw Chang [:kershaw]]

Attachment: crash_log.zip

Reproduce this crash with the following steps:

1. Click + to upload a file on discord.com
2. Rename the uploaded file.
3. Type anything and click enter to send this message.

The attached file is the http log.

Comment 4

[Wayne Mery (:wsmwk)]

I'm surprised this isn't flagged as a regression

Comment 5

[Dragana Damjanovic [:dragana]]

This is not really a regression. There is no impact on the users. This is a diagnostic assert to make sure we behave nice.

Look at the log in comment 3, this is probably a false positive.

Comment 6

[Magnus Melin [:mkmelin]]

(In reply to Dragana Damjanovic [:dragana] from comment #5)

This is not really a regression. There is no impact on the users. This is a diagnostic assert to make sure we behave nice.

But this type of assert takes down non-debug builds. Crash is a pretty hard impact on users.
Can we change the type of assert, or remove it until it's not crashing beta and nightly builds? https://hg.mozilla.org/releases/mozilla-beta/file/8426c57c6960df31cae39214a8a1af0d92170662/netwerk/protocol/http/Http3Stream.cpp#l408

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1750587

Comment 8

[Dragana Damjanovic [:dragana]]

A non-zero Content-Length header is set on nsHttpChannel by the creator of the channel. The creator of the channel does not supply an upload stream.
We should not send a Content-Length header without a body.
A Content-Length header value should match the size of the body.

Necko cannot easily detect this situation and I think it should be fixed by the channel creator

I have created bug 1757910 to fix the problem and I will change this assertion to a debug one because we cannot detect this case correctly.

Comment 9

[Dragana Damjanovic [:dragana]]

Attachment: Change the diagnostic assert to a debug assert until bug 1757910 is fixed.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7eb977e0793
Change the diagnostic assert to a debug assert until bug 1757910 is fixed. r=necko-reviewers,kershaw

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a7eb977e0793