pro.beefree.io - blank page after 1259822 landed

RESOLVED FIXED

Status

RESOLVED FIXED
7 months ago
a month ago

People

(Reporter: miketaylr, Assigned: miketaylr)

Tracking

(Blocks: 1 bug, {regression, site-compat})

Firefox 63
regression, site-compat

Firefox Tracking Flags

(firefox62 unaffected, firefox63+ fixed, firefox64 fixed)

Details

(Whiteboard: [webcompat])

(Assignee)

Description

7 months ago
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.
(Assignee)

Comment 1

7 months ago
:arai, any ideas why updating the error message would break the site? The page doesn't throw in 62.
Flags: needinfo?(arai.unmht)
(Assignee)

Updated

7 months ago
status-firefox62: --- → unaffected
status-firefox63: --- → affected
status-firefox64: --- → affected
(Assignee)

Updated

7 months ago
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)
😭
tracking-firefox63: ? → +
(Assignee)

Comment 9

6 months ago
(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.
(Assignee)

Comment 10

6 months ago
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.
(Assignee)

Comment 11

6 months ago
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

6 months ago
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
(Assignee)

Comment 13

6 months ago
Wow, thanks for the quick fix Roberto! I can confirm this is working in 63+.
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox63: affected → fixed
status-firefox64: affected → fixed
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: → bug 1498257
Component: Desktop → Desktop
Product: Tech Evangelism → Web Compatibility
You need to log in before you can comment on or make changes to this bug.