New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@match dot character matches any character #1899

Closed
zixaphir opened this Issue Mar 25, 2014 · 8 comments

Comments

Projects
None yet
4 participants
@zixaphir
Copy link

zixaphir commented Mar 25, 2014

Steps to reproduce: have a dot in the metadata @match block.

eg,

// ==UserScript==
// @name         match block test
// @match        *://*.4chan.org/*
// ==/UserScript==

console.log('hi');

And then go to a site with a similar name that shouldn't be matched, like http://1d4chan.org/

Expected Result: Nothing should be echoed on the clipboard.

Actual Result: 'hi' is echoed on the clipboard.

I also note that the documentation of match patterns state that this should not be the case:
https://developer.chrome.com/extensions/match_patterns
http://wiki.greasespot.net/Metadata_Block#.40match

@arantius arantius added this to the 1.16 milestone Mar 25, 2014

Ventero added a commit to Ventero/greasemonkey that referenced this issue Mar 25, 2014

@Ventero

This comment has been minimized.

Copy link
Contributor

Ventero commented Mar 25, 2014

Looks like when the match pattern is created, *. in the hostname part is replaced by * 1 before being passed on to GM_convert2RegExp, which in turn generates .* as part of the resulting RegExp. What actually should be generated is something like (.*\.)?.

A possible fix for this - albeit a bit ugly - can be found at Ventero@1e7060a.

@jerone

This comment has been minimized.

Copy link
Contributor

jerone commented Mar 25, 2014

Why does *. need to be converted to * ?

@Ventero

This comment has been minimized.

Copy link
Contributor

Ventero commented Mar 26, 2014

I suppose the idea was that by replacing *. with * in the hostname, GM_convert2RegExp would then turn the * into .*, which can be used to match a domain and all subdomains - but as described in the initial report, this also matches domains that contain the original domain as suffix. So instead, to handle *. properly, something like (.*\.)? or .*\.? should be used to only allow actual subdomains.

@jerone

This comment has been minimized.

Copy link
Contributor

jerone commented Mar 26, 2014

Please correct me if I'm wrong, but I think the part host.replace(/^\*\./, "*") isn't needed. Function GM_convert2RegExpInner will replace the * by .* and replace the following . to \., resulting in .*\..

@Ventero

This comment has been minimized.

Copy link
Contributor

Ventero commented Mar 26, 2014

But then the resulting regexp will not match example.org if the initial host portion was *.example.org, which according to the Match Pattern docs, it should.

@jerone

This comment has been minimized.

Copy link
Contributor

jerone commented Mar 26, 2014

I wouldn't ignore the GM_convert2RegExp. An alternative suggestion would be to change the GM_convert2RegExpInner:

-return res + "$";
+res += "$";
+return res.replace("^.*", "^(.*\\.)?");
@Ventero

This comment has been minimized.

Copy link
Contributor

Ventero commented Mar 26, 2014

That breaks @include *example.org/*, as the original convert function turns that into ^.*example\.org/.*$, whereas your modified version returns ^(.*\.)?example\.org/.*$. The original RegExp would match e.g. http://example.org/, whereas your version doesn't (since it requires the URL to either start with example.org or contain a . in front of it).

@arantius

This comment has been minimized.

Copy link
Collaborator

arantius commented Mar 26, 2014

It's unfortunate that Greasemonkey doesn't have unit tests. This is a case where they would clearly be called for. But good work everybody collaboratively figuring out all the cases (which we currently must test manually).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment