oyvindkinsey/easyXDM

getLocation error with non standard origins

matmiz opened this issue · 10 comments

matmiz commented

Hi,
We are experiencing a lot of errors in the getLocation function (when coming from _getOrigin function).
In many cases, the origin of the event is chrome-extension://..., file:// , chrome://browser, about: etc.

When the origin is such a string, the match function returns null, and an error is thrown when it tries to get a member of a null object.
Is there any solution to this case?
for example , check already in the _getOrigin function for validity of the event.origin

Thanks

matmiz commented

Ok, so I've found the root cause of some of these issues.
There's a chrome extensions called "Buffer" which sends message events.
Easy XDM listens to these messages , and then tries to perform the regular flow, but the origin is chrome-extension://noojglkidnpfjbincgijbaiedldjfbhh , thus the failure in getLocation.

I was wondering what are the consequences for returning empty string when the origin is not a valid URL.

Any thoughts?

oyvindkinsey commented

This seems like a fair thing to support, but generally I'd prefer the callers to instead explicitly check for null and do something sane.

Want to provide a PR?

matmiz commented

Thanks.
I agree, but this extension is just one example.
BTW, the origin is not null, the null is the result of the match function.

I will provide a PR soon

dtomack commented

We've run into this same issue. Is there a timeframe for this to be addressed? Thanks.

(Also, the call stack for the error is completely self-contained within easyXDM. The top is _window_onMessage. I'm unclear how "callers would explicitly check for null...". I don't see any way we can resolve this issue outside of an update to EasyXDM.)

oyvindkinsey commented

@dtomack I would recommend that someone with a good example put up a PR for this issue. I'm happy to review and accept.

matmiz commented

@dtomack

callers would explicitly check for null...". I don't see any way we can resolve this issue outside of an update to EasyXDM.)

I couldn't agree more.
I've put up a PR, hope it is good enough (this is how I solved it in my local code)
#280 .

@oyvindkinsey , This is my first contribution to an open source, be easy on me :)

dtomack commented

@oyvindkinsey @matmiz Hi all, I see the PR was approved a while ago. What are the next steps for getting this out in a release? I just want to know what the timeframe is for seeing a fixed version. Thanks.

  • 👍1
matmiz commented

Hi @dtomack , I think that it's in the hands of @oyvindkinsey right now, there's nothing I can do

oyvindkinsey commented

Closing this as the pr was merged to master.