Bug 239651 – [GLib] Make WebKitSettings XSS auditor functions no-op

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

[GLib] Make WebKitSettings XSS auditor functions no-op

[Zan Dobersek]

Created attachment 458135 [details]
Patch

[Adrian Perez]

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 🤔️

[Zan Dobersek]

Comment on attachment 458135 [details]
Patch

Clearing flags on attachment: 458135

Committed r293215 (249884@trunk): <https://commits.webkit.org/249884@trunk>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/92304443>

[Michael Catanzaro]

Would be good to deprecate these too

[Michael Catanzaro]

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.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 239772

[Michael Catanzaro]

Actually I'll just do that here, it's quick/easy.

[Michael Catanzaro]

(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.

[Michael Catanzaro]

https://github.com/WebKit/WebKit/pull/389

[EWS]

Committed r293507 (250038@main): <https://commits.webkit.org/250038@main>

Reviewed commits have been landed. Closing PR #389 and removing active labels.

[Philippe Normand]

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.

[Michael Catanzaro]

(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.

[Michael Catanzaro]

(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.

[Michael Catanzaro]

Pull request: https://github.com/WebKit/WebKit/pull/441

[EWS]

Committed r293660 (250164@main): <https://commits.webkit.org/250164@main>

Reviewed commits have been landed. Closing PR #441 and removing active labels.

[Michael Catanzaro]

I forgot to deprecate the property WebKitSettings:enable-xss-auditor, because I am incompetent. I think we all knew that already, though. ;)

[Michael Catanzaro]

Bug #240993.