1900339 - Textures don't work when html part is in plain text and texture in zstd

Status: RESOLVED FIXED

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

Description

[Jakub Juszczakiewicz]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:126.0) Gecko/20100101 Firefox/126.0

Steps to reproduce:

Create html5 page with canvas and WebGL JS script with textures.
Share .html + .js + .png on http server, where .html page (.php) was serve as dynamic content without compression, and .js and .png on http server with zstd compression. CORS was configure OK.

Actual results:

HTTP 200 for whole content. No error in console. 3D image was empty (like transparent textures) when .pngs was server with zstd.
After move .png for non compressed share - start working.

Expected results:

HTTP 200 for whole content. No error in console. 3D image works well.
Not need to move .png for non compressed share.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::JavaScript Engine' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Mayank Bansal]

Can you provide a test page that repros this issue for you?

Comment 3

[Jakub Juszczakiewicz]

Working now version is available here:
https://kryptopol.pl/project.en.php
When link to logo_1.png and logo_2.png will be change in logo3d.js for https://www.kryptopol.pl/... it stop working. I will prepare dedicated test in a moment and share link here.

Comment 4

[Jakub Juszczakiewicz]

https://kryptopol.pl/ff_zstd.html <- here is version that works on e.g. chromium, but Firefox don't want to.

Comment 5

[Mayank Bansal]

Thanks for reporting and creating the testcase!

Bisection:
Bug 1871963: Mark fetch zstd wpt tests as passing r=necko-reviewers,valentin

Differential Revision: https://phabricator.services.mozilla.com/D205483

Comment 6

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

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

Comment 7

[Jakub Juszczakiewicz]

Few more info about shared test environment:
zstd in cache, compressed by: zstd --no-progress -19
kit-static-scgi serve cache file through nginx (sources here: https://opensource.krypto-it.pl/sources/kit-static-scgi)
here: https://kryptopol.pl/ff_zstd.html and here: https://www.kryptopol.pl/ff_zstd.html are the same HTML file (first raw, second with compression) - results are similar now
There is not 100% reproduction on Firefox: after F12 -> disable cache and F5 many times, sometime one of file start working, but... on chromium (125.0.6422.141, 64bits) always works.
One more thing: F12 -> Network -> one of file with zstd -> response -> with zstd is crap, but files works on regular view, e.g. images. Should I report next bug for that?

Comment 8

[brunoais]

(In reply to Jakub Juszczakiewicz from comment #4)

https://kryptopol.pl/ff_zstd.html <- here is version that works on e.g. chromium, but Firefox don't want to.

I'm using 127.0b9 and cannot reproduce. The canvas animation shows properly and animates. Is it fixed or did I miss something?

Comment 9

[Jakub Juszczakiewicz]

Attachment: screenshot.png

Here is screen shot with this animation on Firefox 126.0.1 (64bit) @ MX-Linux. I don't see anything other then background, but after for refresh by F5 sometimes one of those 2 textures is visible. On regular page this works well (destination PNGs have different location, without zstd compression). I checked this html in chromium for compare and it's works well. I didn't tested it with newer Firefox yet, because of upgrades in distro.

Comment 10

[Jakub Juszczakiewicz]

Attachment: ss_ff_127_64b.png

I got FF upgrade to 127 and it's the same. On screen shot I got 1 of 2 pngs after few reloads. And don't see responds status.

Comment 11

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

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

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

jesup or valentin, can one of you take a look and see if this connects with the work in bug 1871963? Thanks!

Comment 13

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

Definitely related to zstd. If I remove zstd from network.http.accept-encoding.secure the page starts working.
Interestingly, it also works when loaded from cache.

Comment 14

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

The HTTP channel gets cancelled with NS_IMAGELIB_ERROR_FAILURE
LogMessages — (nsHttp) nsHttpChannel::Cancel [this=5a887d85c420 status=80540005, reason=imgRequest::ContinueCancel]

See this profile here: https://share.firefox.dev/3KCYpwZ

Tim, do you have any insights here?

Comment 15

[Timothy Nikkel (:tnikkel)]

When imagelib tries to pull data from the stream we hit this

https://searchfox.org/mozilla-central/rev/346ac9335cbc33e3124a4541c938a0dc455e785e/image/SourceBuffer.cpp#475

because we read zero bytes. That check was added in bug 1411506. And this bubbles up through RasterImage::OnImageDataAvailable to imgRequest::OnDataAvailable where we hit this cancel

https://searchfox.org/mozilla-central/rev/346ac9335cbc33e3124a4541c938a0dc455e785e/image/imgRequest.cpp#1076

Sorry to bounce the needinfo back at you, but you might be able to tell more quickly then me what the next steps are for getting to the bottom of this.

Comment 16

[Timothy Nikkel (:tnikkel)]

Just loading https://www.kryptopol.pl/logo_1.png seems to be enough to trigger the bug. However loading a page with an img pointing to that url works.

Comment 17

[Valentin Gosu [:valentin] (he/him) {{ FYI: OOO 14 June - 4 August }}]

(In reply to Timothy Nikkel (:tnikkel) from comment #15)

When imagelib tries to pull data from the stream we hit this

https://searchfox.org/mozilla-central/rev/346ac9335cbc33e3124a4541c938a0dc455e785e/image/SourceBuffer.cpp#475

because we read zero bytes. That check was added in bug 1411506. And this bubbles up through RasterImage::OnImageDataAvailable to imgRequest::OnDataAvailable where we hit this cancel

I'm thinking the zstd decoder might be emitting OnDataAvailables with 0 bytes?
Randell, could you take a look?

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

It certainly could be, I'll check

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 1900339: don't send 0-byte OnDataAvailables from http decompression r=nika!

Comment 20

[Randell Jesup [:jesup] (needinfo me)]

We'll want to uplift this to beta

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38f295a7c59a
don't send 0-byte OnDataAvailables from http decompression r=nika

Comment 22

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 1900339: don't send 0-byte OnDataAvailables from http decompression r=nika!

Original Revision: https://phabricator.services.mozilla.com/D213720

Comment 23

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Bug 1900339: don't send 0-byte OnDataAvailables from http decompression r=nika!

Original Revision: https://phabricator.services.mozilla.com/D213720

Comment 24

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Unknown number of broken sites
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Load image from comment in bug
• Risk associated with taking this patch: Super low
• Explanation of risk level: Just adds an early return if there's no data to pass along
• String changes made/needed: none
• Is Android affected?: yes

Comment 25

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/38f295a7c59a

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/730cc8f3c0e4