Bug 171058

Summary: [GTK] After upgrading glib and glib-networking, resources with zero bytes are always identified as text/plain
Product: WebKit Reporter: Carlos Alberto Lopez Perez <clopez>
Component: WebKitGTKAssignee: Claudio Saavedra <csaavedra>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, commit-queue, csaavedra, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugzilla.gnome.org/show_bug.cgi?id=755795
Bug Depends on:    
Bug Blocks: 170942    
Attachments:
Description Flags
Patch none

Description Carlos Alberto Lopez Perez 2017-04-20 09:21:31 PDT
On r215556 <https://trac.webkit.org/r215556> we upgraded glib from 2.44.1 to 2.52.1 and glib-networking from 2.41.4 to 2.50.0

This causes failures on the following tests:


Regressions: Unexpected text-only failures (13)
  fast/preloader/document-write-noscript.html [ Failure ]
  fast/preloader/document-write.html [ Failure ]
  fast/preloader/first-base-tag-scanned-wins.html [ Failure ]
  fast/preloader/first-base-tag-wins.html [ Failure ]
  fast/preloader/image-srcset.html [ Failure ]
  fast/preloader/image.html [ Failure ]
  fast/preloader/input.html [ Failure ]
  fast/preloader/link.html [ Failure ]
  fast/preloader/noscript.html [ Failure ]
  fast/preloader/script.html [ Failure ]
  fast/preloader/style.html [ Failure ]
  fast/preloader/understands-base-tag.html [ Failure ]
  tables/mozilla/core/col_widths_fix_autoFixPer.html [ Failure ]


This are the diffs for all this tests: http://sprunge.us/GZPB?diff

As you can see, the resource gives always now "MIME type text/plain" instead of the MIME type of the extension of the file.

Upon further investigation it turns out, all this files are empty!

$ ls -1s LayoutTests/fast/preloader/resources/script1.js LayoutTests/fast/preloader/resources/noscript-image2.png LayoutTests/fast/preloader/resources/base-image3.png LayoutTests/fast/preloader/resources/base-image2.png LayoutTests/fast/preloader/resources/image1.png LayoutTests/fast/preloader/resources/base-image1.png LayoutTests/fast/preloader/resources/image1.png LayoutTests/fast/preloader/resources/link1.css LayoutTests/fast/preloader/resources/noscript-image2.png LayoutTests/fast/preloader/resources/script1.js LayoutTests/fast/preloader/resources/style1.css LayoutTests/fast/preloader/resources/base-image1.png
0 LayoutTests/fast/preloader/resources/base-image1.png
0 LayoutTests/fast/preloader/resources/base-image1.png
0 LayoutTests/fast/preloader/resources/base-image2.png
0 LayoutTests/fast/preloader/resources/base-image3.png
0 LayoutTests/fast/preloader/resources/image1.png
0 LayoutTests/fast/preloader/resources/image1.png
0 LayoutTests/fast/preloader/resources/link1.css
0 LayoutTests/fast/preloader/resources/noscript-image2.png
0 LayoutTests/fast/preloader/resources/noscript-image2.png
0 LayoutTests/fast/preloader/resources/script1.js
0 LayoutTests/fast/preloader/resources/script1.js
0 LayoutTests/fast/preloader/resources/style1.css


$ file LayoutTests/fast/preloader/resources/script1.js LayoutTests/fast/preloader/resources/noscript-image2.png LayoutTests/fast/preloader/resources/base-image3.png LayoutTests/fast/preloader/resources/base-image2.png LayoutTests/fast/preloader/resources/image1.png LayoutTests/fast/preloader/resources/base-image1.png LayoutTests/fast/preloader/resources/image1.png LayoutTests/fast/preloader/resources/link1.css LayoutTests/fast/preloader/resources/noscript-image2.png LayoutTests/fast/preloader/resources/script1.js LayoutTests/fast/preloader/resources/style1.css LayoutTests/fast/preloader/resources/base-image1.png
LayoutTests/fast/preloader/resources/script1.js:          empty
LayoutTests/fast/preloader/resources/noscript-image2.png: empty
LayoutTests/fast/preloader/resources/base-image3.png:     empty
LayoutTests/fast/preloader/resources/base-image2.png:     empty
LayoutTests/fast/preloader/resources/image1.png:          empty
LayoutTests/fast/preloader/resources/base-image1.png:     empty
LayoutTests/fast/preloader/resources/image1.png:          empty
LayoutTests/fast/preloader/resources/link1.css:           empty
LayoutTests/fast/preloader/resources/noscript-image2.png: empty
LayoutTests/fast/preloader/resources/script1.js:          empty
LayoutTests/fast/preloader/resources/style1.css:          empty
LayoutTests/fast/preloader/resources/base-image1.png:     empty


And for the test tables/mozilla/core/col_widths_fix_autoFixPer.html it turns out also this test is just an empty html

$ ls -s1 LayoutTests/tables/mozilla/core/col_widths_fix_autoFixPer.html
0 LayoutTests/tables/mozilla/core/col_widths_fix_autoFixPer.html


$ file LayoutTests/tables/mozilla/core/col_widths_fix_autoFixPer.html
LayoutTests/tables/mozilla/core/col_widths_fix_autoFixPer.html: empty


That explains the diff on this test also.
Comment 1 Carlos Alberto Lopez Perez 2017-04-20 09:47:09 PDT
Expectations updated at https://trac.webkit.org/changeset/215556
Comment 2 Carlos Alberto Lopez Perez 2017-04-20 09:47:38 PDT
(In reply to Carlos Alberto Lopez Perez from comment #1)
> Expectations updated at https://trac.webkit.org/changeset/215556

 ^^^ I mean at: http://trac.webkit.org/changeset/215561
Comment 3 Carlos Garcia Campos 2017-04-21 00:02:01 PDT
I think this was broken by this commit 

https://git.gnome.org/browse/glib/commit/?id=202a9c3497e0c0b5789e533509dd8671617ae20c

Before this commit the empty file check was done after the g_content_type_guess and only when guessing from path returned an uncertain result. Now the check is done before unconditionally, so there's no basename guess for empty files.
Comment 4 Carlos Garcia Campos 2017-04-21 00:39:38 PDT
Reading the comments in the GNOME bug it seems the change in behavior was accidental, but then they decided it was an improvement. I kind of agree that a  zero size file with .png extension is not a png file (or any other extension). I bet this only affects unit tests, so we can work around the issue inside webkit, either patching glib to only check zero-sized files after guessing the content type form the filename if the result is uncertain, or using real files (small enough, but real) in the tests.
Comment 5 Carlos Alberto Lopez Perez 2017-04-21 03:30:19 PDT
(In reply to Carlos Garcia Campos from comment #4)
> Reading the comments in the GNOME bug it seems the change in behavior was
> accidental, but then they decided it was an improvement. I kind of agree
> that a  zero size file with .png extension is not a png file (or any other
> extension). I bet this only affects unit tests, so we can work around the
> issue inside webkit, either patching glib to only check zero-sized files
> after guessing the content type form the filename if the result is
> uncertain, or using real files (small enough, but real) in the tests.

I wonder why this files are empty on our Layout tests.

Is that intentional? Or it is a mistake nobody noticed so far?
Looking at the git history of this file nothing seems to suggest that the files are empty on purpose. It looks like a mistake.

Maybe we should replace the empty files with files valid for their supposed mime-type (for example, for a png file we can use a 1x1 transparent png).

At the same time we can add a new test that loads several zero-byte resource files. This test should produce plain/text on glib based ports and other thing on the rest of ports.
Comment 6 Carlos Garcia Campos 2017-04-21 04:19:40 PDT
I guess they are empty because the actual contents doesn't really matter. So, making them valid for their mime type would be harmless, and would fix these tests for the glib based ports.
Comment 7 Michael Catanzaro 2017-04-21 06:49:53 PDT
I would vote for changing the tests to not use empty files. This is a silly reason to carry a downstream patch forever and it's not advantageous for our tests to exhibit special behavior that never occurs in releases.
Comment 8 Claudio Saavedra 2017-05-12 04:21:20 PDT
Created attachment 309888 [details]
Patch
Comment 9 Carlos Garcia Campos 2017-05-12 05:30:01 PDT
Comment on attachment 309888 [details]
Patch

Thanks!
Comment 10 WebKit Commit Bot 2017-05-12 05:58:47 PDT
Comment on attachment 309888 [details]
Patch

Clearing flags on attachment: 309888

Committed r216762: <http://trac.webkit.org/changeset/216762>
Comment 11 WebKit Commit Bot 2017-05-12 05:58:49 PDT
All reviewed patches have been landed.  Closing bug.