1490772 - pro.beefree.io - blank page after 1259822 landed

Status: RESOLVED FIXED

(Web Compatibility :: Site Reports, defect)

Description

[Mike Taylor [:miketaylr]]

Originally reported via webcompat.com @ https://github.com/webcompat/web-bugs/issues/18757

STR:

1. navigate to https://pro.beefree.io/login

Expected:
Login page renders

Actual:
Blank page, with the following error message in the console:

TypeError: e.user.subscriptions is undefined, can't access property 0 of it[Learn More]

Mozregression led me here:

13:23.49 INFO: Narrowed inbound regression window from [5d6bf031, f433ed4f] (3 builds) to [5d6bf031, f0c6e521] (2 builds) (~1 steps left)
13:23.49 INFO: No more inbound revisions, bisection finished.
13:23.49 INFO: Last good revision: 5d6bf0312e088bb2424a8177589a9fd4aa44bfa8
13:23.49 INFO: First bad revision: f0c6e521429cfaff0585ec6eaf734e9fcf873f8a
13:23.49 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5d6bf0312e088bb2424a8177589a9fd4aa44bfa8&tochange=f0c6e521429cfaff0585ec6eaf734e9fcf873f8a

[Tracking Requested - why for this release]: Site is broken now, works in 62 and below.

Comment 1

[Mike Taylor [:miketaylr]]

:arai, any ideas why updating the error message would break the site? The page doesn't throw in 62.

Comment 2

[Tooru Fujisawa [:arai]]

still investigating, but I see a bunch of tests against Error#message in the code there.

Comment 3

[Tooru Fujisawa [:arai]]

this looks suspicious
  https://github.com/csnover/TraceKit/blob/f03357cf005df5de299fddd22f828bbd851ecdba/tracekit.js#L785
the compressed code matches to the structure and literals.

Comment 4

[Tooru Fujisawa [:arai]]

looks like the above one is not related to this regression.
there's yet another one that checks error message.

this matches to the compressed code (not sure if this is the primary source tho, I cannot find anything else on github)
  https://github.com/ignivamanishbhudhraja/React-Native-Boilerplate/blob/83bc40ca1e549ae4dd12ec23db1b5fddd2d84a56/src/utilities/Idx.js#L110

Comment 5

[Tooru Fujisawa [:arai]]

Okay, figured out the issue.

here's the code from https://pro.beefree.io/static/js/main.v18.09.03.0946.js (prettified, and modified a bit, for readability)

        l = new Function(
"$object$", `
  try {
    $object$.$property$;
  } catch (error) {
    return new RegExp(
      error.message
        .replace(/[-\\[\\]\\/\\{\\}\\(\\)\\*\\+\\?\\.\\\\\\^\\$\\|]/g,'\\\\$&')
        .replace('\\\\$object\\\\$', '.+')
        .replace('\\\\$property\\\\$', '.+');
    );
  }
  throw new Error('Expected property access on ' + $object$ + ' to throw.');
`);


it's a function that dynamically creates a pattern that is supposed to match error message for property access on null/undefined.

it's called like the following, to make patterns for null and undefined
  * l(null)
  * l(void 0)

then, it creates the following pattern:
  * /.+ is null, can't access property ".+" of it/
  * /.+ is undefined, can't access property ".+" of it/

then, this is wrong because it contains double quotation, which is part of property expression,
which means it doesn't match to numeric property.

and the failure happens in the following line:

  return e.user.subscriptions[0]

where e.user.subscriptions is undefined, and property expression is number 0, which throws the following error:

  e.user.subscriptions is undefined, can't access property 0 of it

and it doesn't contain double quotation.

Comment 6

[Tooru Fujisawa [:arai]]

then, the above patterns are used like the following:

    function a(e, t) {
        try {
            return t(e)
        } catch (e) {
            if (e instanceof TypeError) {
                if (r(e)) return null;
                if (o(e)) return
            }
            throw e
        }
    }

    function r(e) {
        var t = e.message;
        i || (i = l(null));
        return i.test(t)
    }

    function o(e) {
        var t = e.message;
        s || (s = l(void 0));
        return s.test(t)
    }

the error happens inside function t, callsed inside function a,
and function a catches the exception, and if it matches to the property access on null/undefined, it returns.

in this case, the pattern is incomplete and the error is thrown again.

Comment 7

[Tooru Fujisawa [:arai]]

basically, the content of Error.prototype.message is not spec-ed, and People shouldn't use its programatically, especially for branch condition.
I'd suggest changing this as tech evangelism issue, unless it's from a library widely used and it impacts much.

(if it affects many websites, we could tweak the error message a bit, in order to make the pattern match)

jorendorff, can I have your opinion?

Comment 8

[Jan de Mooij [:jandem]]

😭

Comment 9

[Mike Taylor [:miketaylr]]

(In reply to Tooru Fujisawa [:arai] from comment #7)
> I'd suggest changing this as tech evangelism issue, unless it's from a
> library widely used and it impacts much.

Thanks arai! I'd recommend we try some outreach before moving the bug -- and we'll keep an eye out for more bustage like this. 

Also, blargh.

Comment 10

[Mike Taylor [:miketaylr]]

I've attempted contact via https://beefree.io/get-in-touch/, if they don't respond in a few days we'll try other means.

Comment 11

[Mike Taylor [:miketaylr]]

Update:

> Thanks for your message... I am forwarding it to our BEE Pro developers for review and someone will get back to you with more information about the specific code on the BEE Pro login page.

Comment 12

[rmarchet]

I'm Roberto Marchetti, frontend developer of BEE Pro.
We published an updated version of our frontend with the fix.
Thank you for your notification of the problem
Roberto Marchetti

Comment 13

[Mike Taylor [:miketaylr]]

Wow, thanks for the quick fix Roberto! I can confirm this is working in 63+.

Comment 14

[Jason Orendorff [:jorendorff]]

Wow. Great work here, everyone. I hope this holds up.

Note that we are changing that error message *again* in the next version, based on user feedback. See bug 1488417 or try a Firefox Nightly build.

:-|

Comment 15

[Kohei Yoshino [:kohei]]

Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/improved-javascript-error-message-breaks-code-relying-on-the-legacy-format/