Stacks.js: Ensure arrays are treated as such in zone-file dependency to prevent file parsing errors

[markmhx]

I'm encountering the following error thrown by the zone-file dependency upon passing a username ("markmhendrickson.id" in my case) into getFile (see my invocation code here):

TypeError: Cannot read property 'length' of undefined
    at flatten (parseZoneFile.js:33)
    at parseZoneFile (parseZoneFile.js:9)
    at profileZoneFiles.js:76
    at new Promise (<anonymous>)
    at resolveZoneFileToProfile (profileZoneFiles.js:73)
    at profileLookup.js:38

The flatten method in that package (https://github.com/blockstack/zone-file-js/blob/master/src/parseZoneFile.es6#L16) seems to think that regex matches have been found in the text variable (which represents my zonefile correctly). So it enters the for-loop on line 31 before trying to get the length of match[0] which is undefined since captured is actually an empty array.

Note that when I omit the "username" parameter from the "getFile" method, I don't encounter this error.

I can't figure out what's going on here. I thought maybe this was the result of not applying "publish_data" as a scope during authentication, but the same error persists even after adding it and re-authenticating with the same ID.

This can also be tested for recreation by going through the authentication flow on https://humans.name/ and viewing the console upon callback.

@kantai any ideas given your familiarity with the zone-file package?

[kantai]

@markmhx -- I can't recreate this bug at the moment -- if I run this, for example:

var zf = require('zone-file')
fetch('https://core.blockstack.org/v1/names/markmhendrickson.id')
  .then(x => x.json()).then(x => x.zonefile)
  .then(zonefile => console.log(zf.parseZoneFile(z.zonefile)))

It looks like it's correctly parsing your zone file. I also don't get that error on https://humans.name/markmhendrickson.id (I get a 404 because it's trying to read a file that doesn't exist), and if you look at https://humans.name/blankstein.id that seems to work.

Any chance you have the actual value of the text variable in the error you're seeing?

[markmhx]

Ack sorry, I had commented out username usage in production on humans.name Can you try testing again to recreate? I just re-introduced its usage.

[kantai]

So it looks like this issue is caused by the way zone-file-js is iterating over various objects combined with the use of a library which modifies Array.prototype

[markmhx]

Ember is presumably the culprit here with its prototype extensions. I'll look into a solution, perhaps modifying zone-file-js to play more nicely if there isn't a clean way to control Ember's extensions behavior.

[markmhx]

Also, any reason not to just replace that for-in loop on line 31 with forEach?

It does seem as though the use of for-in here is questionable given that it's meant for capturing object properties rather than array elements per se.

[kantai]

Yeah, definitely agree -- I started a branch to refactor that library so that it's structured similarly to our other libraries. Just using flow typing would have caused those uses of for-in to report errors.

[markmhx]

@moxiegirl you sure this is an enhancement not bug? 🤔

[moxiegirl]

From your discussion, it appears this isn't a bug but rather the way the library was coded doesn't play well with a EmberJS environment. So, it isn't a bug in our code but we can enhance our code so it works with EmberJS

[markmhx]

The EmberJS environment surfaced this but I believe we essentially concluded that the underlying dependency isn't handling arrays properly, in that it's iterating through them using technique that should be reserved for object properties.

[moxiegirl]

@markmhx If that's the case, you would adjust the description to reflect the actual issue being addressed. Still, it was working with other environments, so it isn't really a bug. It doesn't downgrade the issue to not call it a bug.

[markmhx]

I mean, no big deal either way. Title updated in any case.

[kantai]

Here's the PR on zone-file:

https://github.com/blockstack/zone-file-js/pull/5

[markmhx]

@kantai Awesome! Thanks for jumping on this so quickly. What's next? Review, merge, blockstack.js dependency bump and release?

[kantai]

Yep, once that's reviewed, I'll release zone-file-js. Then the dependency can be upgraded, tested in blockstack.js.

[kantai]

zone-file-js was released at 1.0.0 --- to include that in blockstack.js, the dependency will need to be updated.

[markmhx]

@kantai Want me to upgrade the dependency? It could be my first blockstack.js PR! 🤗

[kantai]

Yep, please do!