Add size message checks for logging #45892

pvollan · 2025-05-25T20:17:57Z

`3a08604`

Add size message checks for logging
https://bugs.webkit.org/show_bug.cgi?id=293558
rdar://152010794

Reviewed by Sihui Liu.

* Source/WebKit/Shared/LogStream.cpp:
(WebKit::LogStream::logOnBehalfOfWebContent):
* Source/WebKit/Shared/LogStream.h:
* Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:
(WebKit::WebProcess::registerLogHook):

Canonical link: https://commits.webkit.org/296808@main

63eb21a

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-05-25T20:18:03Z

EWS run on previous version of this PR (hash 90e7561)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	⏳ 🧪 win-tests
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	~~🧪 api-ios~~	~~🧪 mac-wk2~~	✅ 🛠 gtk
	❌ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	❌ 🛠 vision-sim	~~🧪 mac-wk2-stress~~	✅ 🧪 api-gtk
	⏳ 🧪 vision-wk2	~~🧪 mac-intel-wk2~~	✅ 🛠 playstation
	❌ 🛠 tv	~~🛠 mac-safer-cpp~~
	❌ 🛠 tv-sim
	❌ 🛠 watch
	❌ 🛠 watch-sim

webkit-early-warning-system · 2025-05-25T20:52:14Z

EWS run on previous version of this PR (hash 303cb0f)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

youennf · 2025-05-28T14:36:56Z

Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm

        if (type == OS_LOG_TYPE_FAULT)
            type = OS_LOG_TYPE_ERROR;

        if (char* messageString = os_log_copy_message_string(msg)) {
            auto logString = unsafeSpan8IncludingNullTerminator(messageString);
-            WebProcess::singleton().sendLogOnStream(logChannel, logCategory, logString, type);
+            if (logString.size() < logStringMaxSize)
+                WebProcess::singleton().sendLogOnStream(logChannel, logCategory, logString, type);


Could this affect WebRTC logging which can be quite long (but quite useful for debugging)?
Also, if a channel log is debug, I would expect logging to be high in general.

That is a good point! Is there WebRTC logging in the WebContent process? If so, would you know the approximate size of the largest log string?

Thanks for reviewing!

In the latest patch, I increased the max size of the log string to accommodate for this.

webkit-early-warning-system · 2025-05-30T14:54:20Z

EWS run on previous version of this PR (hash 6330177)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 win~~
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	~~🧪 win-tests~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

szewai · 2025-05-30T16:15:21Z

Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm

        if (type == OS_LOG_TYPE_FAULT)
            type = OS_LOG_TYPE_ERROR;

        if (char* messageString = os_log_copy_message_string(msg)) {
            auto logString = unsafeSpan8IncludingNullTerminator(messageString);
-            WebProcess::singleton().sendLogOnStream(logChannel, logCategory, logString, type);
+            if (logString.size() < logStringMaxSize)


Should we truncate the string and send instead of dropping message?

That is a good point, will fix!

Thanks for reviewing!

webkit-early-warning-system · 2025-05-30T21:27:28Z

EWS run on previous version of this PR (hash cde89e0)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

szewai · 2025-05-30T22:31:34Z

Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm

+                String logStringTruncated = String::fromUTF8(logString).left(logStringMaxSize - 1);
+                logString = byteCast<uint8_t>(logStringTruncated.utf8().spanIncludingNullTerminator());
+                WebProcess::singleton().sendLogOnStream(logChannel, logCategory, logString, type);
+            } else


Maybe we could move WebProcess::singleton().sendLogOnStream( out of if condition and remove else

That is a good point, will fix.

Thanks for reviewing!

webkit-early-warning-system · 2025-06-28T23:57:06Z

EWS run on previous version of this PR (hash 5cb177a)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	🛠 mac	~~🛠 wpe~~	~~🛠 win~~
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	~~🧪 win-tests~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	~~🧪 api-wpe~~
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	~~🛠 gtk~~
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	~~🧪 api-gtk~~
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-06-30T14:41:30Z

EWS run on current version of this PR (hash 63eb21a)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt		✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

https://bugs.webkit.org/show_bug.cgi?id=293558 rdar://152010794 Reviewed by Sihui Liu. * Source/WebKit/Shared/LogStream.cpp: (WebKit::LogStream::logOnBehalfOfWebContent): * Source/WebKit/Shared/LogStream.h: * Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm: (WebKit::WebProcess::registerLogHook): Canonical link: https://commits.webkit.org/296808@main

webkit-commit-queue · 2025-06-30T17:22:50Z

Committed 296808@main (3a08604): https://commits.webkit.org/296808@main

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

pvollan self-assigned this May 25, 2025

pvollan added the WebKit Process Model Bugs related to WebKit's multi-process architecture label May 25, 2025

pvollan requested review from cdumez and szewai May 25, 2025 20:18

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 25, 2025

pvollan removed the merging-blocked Applied to prevent a change from being merged label May 25, 2025

pvollan force-pushed the eng/Add-size-message-checks-for-logging branch from 90e7561 to 303cb0f Compare May 25, 2025 20:52

youennf reviewed May 28, 2025

View reviewed changes

pvollan requested a review from youennf May 29, 2025 19:12

pvollan force-pushed the eng/Add-size-message-checks-for-logging branch from 303cb0f to 6330177 Compare May 30, 2025 14:54

szewai reviewed May 30, 2025

View reviewed changes

pvollan force-pushed the eng/Add-size-message-checks-for-logging branch from 6330177 to cde89e0 Compare May 30, 2025 21:27

pvollan requested a review from szewai May 30, 2025 21:28

szewai approved these changes May 30, 2025

View reviewed changes

pvollan force-pushed the eng/Add-size-message-checks-for-logging branch from cde89e0 to 5cb177a Compare June 28, 2025 23:57

pvollan force-pushed the eng/Add-size-message-checks-for-logging branch 2 times, most recently from 9898373 to 63eb21a Compare June 30, 2025 14:41

pvollan added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 30, 2025

webkit-commit-queue force-pushed the eng/Add-size-message-checks-for-logging branch from 63eb21a to 3a08604 Compare June 30, 2025 17:22

webkit-commit-queue merged commit 3a08604 into WebKit:main Jun 30, 2025

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add size message checks for logging #45892

Add size message checks for logging #45892

pvollan commented May 25, 2025 •

edited by webkit-early-warning-system

Loading

Uh oh!

webkit-early-warning-system commented May 25, 2025 •

edited

Loading

Uh oh!

webkit-early-warning-system commented May 25, 2025 •

edited

Loading

Uh oh!

youennf May 28, 2025

Uh oh!

pvollan May 28, 2025

Uh oh!

pvollan May 30, 2025

Uh oh!

webkit-early-warning-system commented May 30, 2025 •

edited

Loading

Uh oh!

szewai May 30, 2025

Uh oh!

pvollan May 30, 2025

Uh oh!

webkit-early-warning-system commented May 30, 2025 •

edited

Loading

Uh oh!

szewai May 30, 2025

Uh oh!

pvollan Jun 28, 2025

Uh oh!

webkit-early-warning-system commented Jun 28, 2025 •

edited

Loading

Uh oh!

webkit-early-warning-system commented Jun 30, 2025 •

edited

Loading

Uh oh!

webkit-commit-queue commented Jun 30, 2025

Uh oh!

Uh oh!

Add size message checks for logging #45892

Add size message checks for logging #45892

Conversation

pvollan commented May 25, 2025 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-commit-queue commented Jun 30, 2025

Uh oh!

Uh oh!

pvollan commented May 25, 2025 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented May 25, 2025 •

edited

Loading

webkit-early-warning-system commented May 25, 2025 •

edited

Loading

webkit-early-warning-system commented May 30, 2025 •

edited

Loading

webkit-early-warning-system commented May 30, 2025 •

edited

Loading

webkit-early-warning-system commented Jun 28, 2025 •

edited

Loading

webkit-early-warning-system commented Jun 30, 2025 •

edited

Loading