Closed Bug 1490772 Opened 3 years ago Closed 3 years ago

pro.beefree.io - blank page after 1259822 landed

Categories

(Web Compatibility :: Desktop, defect)

Firefox 63
defect
Not set
normal

Tracking

(firefox62 unaffected, firefox63+ fixed, firefox64 fixed)

RESOLVED FIXED
Tracking Status
firefox62 --- unaffected
firefox63 + fixed
firefox64 --- fixed

People

(Reporter: miketaylr, Assigned: miketaylr)

References

Details

(Keywords: regression, site-compat, Whiteboard: [webcompat])

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.
:arai, any ideas why updating the error message would break the site? The page doesn't throw in 62.
Flags: needinfo?(arai.unmht)
Whiteboard: [webcompat]
Keywords: site-compat
still investigating, but I see a bunch of tests against Error#message in the code there.
this looks suspicious
  https://github.com/csnover/TraceKit/blob/f03357cf005df5de299fddd22f828bbd851ecdba/tracekit.js#L785
the compressed code matches to the structure and literals.
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
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.
Flags: needinfo?(arai.unmht)
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.
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?
Flags: needinfo?(jorendorff)
😭
(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.
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.
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.
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
Wow, thanks for the quick fix Roberto! I can confirm this is working in 63+.
Status: NEW → RESOLVED
Closed: 3 years ago
Component: JavaScript Engine → Desktop
Product: Core → Tech Evangelism
Resolution: --- → FIXED
Version: 63 Branch → Firefox 63
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.

:-|
Flags: needinfo?(jorendorff)
Assignee: nobody → miket
See Also: → 1498257
Product: Tech Evangelism → Web Compatibility
You need to log in before you can comment on or make changes to this bug.