Bug 209521 - App-bound domain checks should provide more debugging details at script evaluation sites
Summary: App-bound domain checks should provide more debugging details at script evalu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: katherine_cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-24 16:38 PDT by katherine_cheney
Modified: 2020-03-25 09:55 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.21 KB, patch)
2020-03-24 16:59 PDT, katherine_cheney
no flags Details | Formatted Diff | Diff
Patch (6.61 KB, patch)
2020-03-25 08:54 PDT, katherine_cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description katherine_cheney 2020-03-24 16:38:48 PDT
This would be helpful for debugging.
Comment 1 katherine_cheney 2020-03-24 16:39:27 PDT
<rdar://problem/60837954>
Comment 2 katherine_cheney 2020-03-24 16:59:55 PDT
Created attachment 394443 [details]
Patch
Comment 3 Chris Dumez 2020-03-24 17:04:07 PDT
Comment on attachment 394443 [details]
Patch

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

> Source/WebCore/bindings/js/ScriptController.cpp:580
> +        m_frame.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user script injection for non-app bound domain.");

Why do release logging here?
How do you know document is non null?

> Source/WebCore/page/Frame.cpp:630
>          return;

Why do release logging here?
Comment 4 katherine_cheney 2020-03-24 17:08:06 PDT
Comment on attachment 394443 [details]
Patch

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

>> Source/WebCore/bindings/js/ScriptController.cpp:580
>> +        m_frame.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user script injection for non-app bound domain.");
> 
> Why do release logging here?
> How do you know document is non null?

I didn't see any other examples of release logging so I wasn't sure if I could here. I'll add a check for m_frame.document().

>> Source/WebCore/page/Frame.cpp:630
>>          return;
> 
> Why do release logging here?

Same as above, I didn't see any other examples of release logging so I wasn't sure if I could here
Comment 5 katherine_cheney 2020-03-24 17:08:49 PDT
(In reply to katherine_cheney from comment #4)
> Comment on attachment 394443 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=394443&action=review
> 
> >> Source/WebCore/bindings/js/ScriptController.cpp:580
> >> +        m_frame.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user script injection for non-app bound domain.");
> > 
> > Why do release logging here?
> > How do you know document is non null?
> 
> I didn't see any other examples of release logging so I wasn't sure if I
> could here. I'll add a check for m_frame.document().
> 

(no other examples in this file)

> >> Source/WebCore/page/Frame.cpp:630
> >>          return;
> > 
> > Why do release logging here?
> 
> Same as above, I didn't see any other examples of release logging so I
> wasn't sure if I could here
Comment 6 Chris Dumez 2020-03-24 17:38:07 PDT
(In reply to katherine_cheney from comment #4)
> Comment on attachment 394443 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=394443&action=review
> 
> >> Source/WebCore/bindings/js/ScriptController.cpp:580
> >> +        m_frame.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user script injection for non-app bound domain.");
> > 
> > Why do release logging here?
> > How do you know document is non null?
> 
> I didn't see any other examples of release logging so I wasn't sure if I
> could here. I'll add a check for m_frame.document().
> 
> >> Source/WebCore/page/Frame.cpp:630
> >>          return;
> > 
> > Why do release logging here?
> 
> Same as above, I didn't see any other examples of release logging so I
> wasn't sure if I could here

I believe you can do release logging anywhere you'd like.
Comment 7 katherine_cheney 2020-03-25 08:54:32 PDT
Created attachment 394502 [details]
Patch
Comment 8 EWS 2020-03-25 09:30:01 PDT
Committed r258986: <https://trac.webkit.org/changeset/258986>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394502 [details].
Comment 9 Simon Fraser (smfr) 2020-03-25 09:55:38 PDT Comment hidden (obsolete)