1242019 - Cut off data: URIs within CSP console messages after 40 chars

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb]]

data: URIs can be long and after hearing complaints from several people I think it makes sense to cut off data: URIs after 40 chars within the CSP console message.

Comment 1

[April King [:April]]

CSP error messaging in FF is largely why I do my CSP testing in Chrome. It would be absolutely fantastic if we could make some improvements in this regards.

Thanks!

Comment 2

[Kate McKinley [:kmckinley, :Kate]]

Attachment: Bug 1242019 Truncate data URIs in CSP log messages.

Review commit: https://reviewboard.mozilla.org/r/78254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/78254/

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8790454 [details]
Bug 1242019 Truncate data URIs in CSP log messages.

https://reviewboard.mozilla.org/r/78254/#review76940

r=me if you address my feedback!

::: dom/security/test/csp/test_bug1242019.html:25
(Diff revision 1)
> +<script class="testbody" type="text/javascript">
> +// Load locale string during mochitest
> +//var stringBundleService = SpecialPowers.Cc["@mozilla.org/intl/stringbundle;1"]
> +//                          .getService(SpecialPowers.Ci.nsIStringBundleService);
> +//var localizer = stringBundleService.createBundle("chrome://global/locale/security/csp.properties");
> +//var confusionMsg = localizer.formatStringFromName("hostNameMightBeKeyword", ["SELF", "self"], 2);

please remove commented out code if not needed.

::: dom/security/test/csp/test_bug1242019.html:33
(Diff revision 1)
> +  SpecialPowers.postConsoleSentinel();
> +  SimpleTest.finish();
> +};
> +
> +// To prevent the test from asserting twice and calling SimpleTest.finish() twice,
> +// startTest will be marked false as soon as the confusionMsg is detected.

that comment seems wrong, copy/paste? please remove

::: dom/security/test/csp/test_bug1242019.html:34
(Diff revision 1)
> +  SimpleTest.finish();
> +};
> +
> +// To prevent the test from asserting twice and calling SimpleTest.finish() twice,
> +// startTest will be marked false as soon as the confusionMsg is detected.
> +startTest = false;

why do you need the startTest variable? I suppose you can remove it, right?

::: dom/security/test/csp/test_bug1242019.html:39
(Diff revision 1)
> +startTest = false;
> +SpecialPowers.registerConsoleListener(function ConsoleMsgListener(aMsg) {
> +  if (startTest) {
> +    // look for the message with data uri and see it is truncated to 40 chars
> +    if (aMsg.message.indexOf("Content Security Policy: ") > -1) {
> +      data_uri = aMsg.message.replace(/^.*(data:\S+)\.{3}.*$/, '$1');

I don't really like that regular expression. Can't you just use the full message within indexOf()? At least you have to add a comment what the replace() function is doing.

::: dom/security/test/csp/test_bug1242019.html:40
(Diff revision 1)
> +SpecialPowers.registerConsoleListener(function ConsoleMsgListener(aMsg) {
> +  if (startTest) {
> +    // look for the message with data uri and see it is truncated to 40 chars
> +    if (aMsg.message.indexOf("Content Security Policy: ") > -1) {
> +      data_uri = aMsg.message.replace(/^.*(data:\S+)\.{3}.*$/, '$1');
> +      console.log('uri = ', data_uri);

please remove the console.log()

::: dom/security/test/csp/test_bug1242019.html:46
(Diff revision 1)
> +      ok(data_uri.length <= 40, "Data URI only shows 40 characters in the console");
> +      SimpleTest.executeSoon(cleanup);
> +    } else {
> +      // don't see the warning yet? wait.
> +      return;
> +    }

no need for the 'else' branch, right?

Comment 4

[Kate McKinley [:kmckinley, :Kate]]

Comment on attachment 8790454 [details]
Bug 1242019 Truncate data URIs in CSP log messages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/78254/diff/1-2/

Comment 5

[Kate McKinley [:kmckinley, :Kate]]

Comment on attachment 8790454 [details]
Bug 1242019 Truncate data URIs in CSP log messages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/78254/diff/2-3/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/566cc2b60a8c
Truncate data URIs in CSP log messages. r=ckerschb

Comment 7

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/566cc2b60a8c