Bug 239651 - [GLib] Make WebKitSettings XSS auditor functions no-op
Summary: [GLib] Make WebKitSettings XSS auditor functions no-op
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords: InRadar
Depends on: 239772
Blocks:
  Show dependency treegraph
 
Reported: 2022-04-22 03:54 PDT by Zan Dobersek
Modified: 2022-05-26 19:12 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.31 KB, patch)
2022-04-22 03:55 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2022-04-22 03:54:29 PDT
[GLib] Make WebKitSettings XSS auditor functions no-op
Comment 1 Zan Dobersek 2022-04-22 03:55:52 PDT
Created attachment 458135 [details]
Patch
Comment 2 Adrian Perez 2022-04-22 04:03:53 PDT
Comment on attachment 458135 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=458135&action=review

> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1824
> +    g_warning("webkit_settings_get_enable_xss_auditor is deprecated and always returns FALSE. XSS auditor is no longer supported.");

It would be nice to use g_warning_once(), but that was added in GLib 2.64
which is newer than our minimum GLib requirement, so let's just leave this
as-is. Anyway, I don't expect any program to have many calls to these
functions, and therefore there shouldn't be logging spam due to this 🤔️
Comment 3 Zan Dobersek 2022-04-22 04:09:12 PDT
Comment on attachment 458135 [details]
Patch

Clearing flags on attachment: 458135

Committed r293215 (249884@trunk): <https://commits.webkit.org/249884@trunk>
Comment 4 Zan Dobersek 2022-04-22 04:09:17 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Radar WebKit Bug Importer 2022-04-25 17:37:34 PDT
<rdar://problem/92304443>
Comment 6 Michael Catanzaro 2022-04-25 17:38:02 PDT
Would be good to deprecate these too
Comment 7 Michael Catanzaro 2022-04-26 09:20:26 PDT
Ah, oops, we actually need to revert this: it prints whenever constructing a WebKitSettings object because the property setter calls these functions.

I don't think we need runtime warnings. It would be sufficient to just deprecate the functions (WEBKIT_API -> WEBKIT_DEPRECATED in the header file, and Deprecated: 2.38 annotation in the source file). I'll follow up on that in bug #239538 if you don't beat me to it.
Comment 8 WebKit Commit Bot 2022-04-26 09:21:33 PDT
Re-opened since this is blocked by bug 239772
Comment 9 Michael Catanzaro 2022-04-26 09:49:30 PDT
Actually I'll just do that here, it's quick/easy.
Comment 10 Michael Catanzaro 2022-04-26 10:13:56 PDT
(In reply to Michael Catanzaro from comment #7)
> Ah, oops, we actually need to revert this: it prints whenever constructing a
> WebKitSettings object because the property setter calls these functions.

Also it looks like this broke >200 API tests, but nobody noticed. The API tests were already in bad shape.
Comment 11 Michael Catanzaro 2022-04-26 10:16:44 PDT
https://github.com/WebKit/WebKit/pull/389
Comment 12 EWS 2022-04-27 06:16:53 PDT
Committed r293507 (250038@main): <https://commits.webkit.org/250038@main>

Reviewed commits have been landed. Closing PR #389 and removing active labels.
Comment 13 Philippe Normand 2022-04-29 02:00:10 PDT
And now we get a bunch of build warnings, can this be avoided?

[1727/2070] Building CXX object Source/WebKit/CMakeFiles/WebKit.dir/UIProcess/API/glib/WebKitSettings.cpp.o
/app/webkit/Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:232:9: warning: 'webkit_settings_set_enable_xss_auditor' is deprecated [-Wdeprecated-declarations]
        webkit_settings_set_enable_xss_auditor(settings, g_value_get_boolean(value));
        ^
/app/webkit/Source/WebKit/UIProcess/API/gtk/WebKitSettings.h:141:55: note: 'webkit_settings_set_enable_xss_auditor' has been explicitly marked deprecated here
__attribute__((visibility("default"))) __attribute__((__deprecated__)) void
                                                      ^
/app/webkit/Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:434:36: warning: 'webkit_settings_get_enable_xss_auditor' is deprecated [-Wdeprecated-declarations]
        g_value_set_boolean(value, webkit_settings_get_enable_xss_auditor(settings));
                                   ^
/app/webkit/Source/WebKit/UIProcess/API/gtk/WebKitSettings.h:138:55: note: 'webkit_settings_get_enable_xss_auditor' has been explicitly marked deprecated here
__attribute__((visibility("default"))) __attribute__((__deprecated__)) gboolean
                                                      ^
2 warnings generated.
[1934/2070] Building CXX object Tools/TestWebKitAPI/glib/CMakeFiles/TestWebKitSettings.dir/__/Tests/WebKitGLib/TestWebKitSettings.cpp.o
/app/webkit/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:76:76: warning: 'webkit_settings_get_enable_xss_auditor' is deprecated [-Wdeprecated-declarations]
    do { if (__builtin_expect (__extension__ ({ int _g_boolean_var_; if (!(webkit_settings_get_enable_xss_auditor(settings))) _g_boolean_var_ = 1; else _g_boolean_var_ = 0; _g_boolean_var_; }), 1)) ; else g_assertion_message (((gchar*) 0), "/app/webkit/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp", 76, ((const char*) (__PRETTY_FUNCTION__)), "'" "webkit_settings_get_enable_xss_auditor(settings)" "' should be FALSE"); } while (0);
                                                                           ^
/app/webkit/WebKitBuild/Release/WebKit2Gtk/Headers/webkit2/WebKitSettings.h:138:55: note: 'webkit_settings_get_enable_xss_auditor' has been explicitly marked deprecated here
__attribute__((visibility("default"))) __attribute__((__deprecated__)) gboolean
                                                      ^
/app/webkit/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:77:5: warning: 'webkit_settings_set_enable_xss_auditor' is deprecated [-Wdeprecated-declarations]
    webkit_settings_set_enable_xss_auditor(settings, (!(0)));
    ^
/app/webkit/WebKitBuild/Release/WebKit2Gtk/Headers/webkit2/WebKitSettings.h:141:55: note: 'webkit_settings_set_enable_xss_auditor' has been explicitly marked deprecated here
__attribute__((visibility("default"))) __attribute__((__deprecated__)) void
                                                      ^
/app/webkit/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:78:76: warning: 'webkit_settings_get_enable_xss_auditor' is deprecated [-Wdeprecated-declarations]
    do { if (__builtin_expect (__extension__ ({ int _g_boolean_var_; if (!(webkit_settings_get_enable_xss_auditor(settings))) _g_boolean_var_ = 1; else _g_boolean_var_ = 0; _g_boolean_var_; }), 1)) ; else g_assertion_message (((gchar*) 0), "/app/webkit/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp", 78, ((const char*) (__PRETTY_FUNCTION__)), "'" "webkit_settings_get_enable_xss_auditor(settings)" "' should be FALSE"); } while (0);
                                                                           ^
/app/webkit/WebKitBuild/Release/WebKit2Gtk/Headers/webkit2/WebKitSettings.h:138:55: note: 'webkit_settings_get_enable_xss_auditor' has been explicitly marked deprecated here
__attribute__((visibility("default"))) __attribute__((__deprecated__)) gboolean
                                                      ^
3 warnings generated.
Comment 14 Michael Catanzaro 2022-04-29 06:50:22 PDT
(In reply to Philippe Normand from comment #13)
> And now we get a bunch of build warnings, can this be avoided?

Oops, not sure how I missed this. Yes, they should be suppressed.
Comment 15 Michael Catanzaro 2022-04-29 07:11:01 PDT
(In reply to Michael Catanzaro from comment #14)
> Oops, not sure how I missed this.

Ah, the answer is: too many other warnings that I haven't fixed yet.
Comment 16 Michael Catanzaro 2022-04-29 11:52:17 PDT
Pull request: https://github.com/WebKit/WebKit/pull/441
Comment 17 EWS 2022-05-02 02:32:41 PDT
Committed r293660 (250164@main): <https://commits.webkit.org/250164@main>

Reviewed commits have been landed. Closing PR #441 and removing active labels.
Comment 18 Michael Catanzaro 2022-05-26 18:05:48 PDT
I forgot to deprecate the property WebKitSettings:enable-xss-auditor, because I am incompetent. I think we all knew that already, though. ;)
Comment 19 Michael Catanzaro 2022-05-26 18:09:17 PDT
Bug #240993.