Skip to content
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

Search terms are not adequately escaped #156

Open
jonathanj opened this issue Sep 18, 2017 · 12 comments
Open

Search terms are not adequately escaped #156

jonathanj opened this issue Sep 18, 2017 · 12 comments

Comments

@jonathanj
Copy link

M-x ggtags-find-tag-dwim ->Decimal produces the following error:

global -v --result=grep --color=always --path-style=shorter -\>Decimal
global: invalid option -- '>'

M-x ggtags-find-tag-dwim *assert* produces a slightly different error, although still a result of inadequate escaping:

global -v --result=grep --color=always --path-style=shorter \*assert\*
global: invalid regular expression.

It's fairly common in Clojure (and maybe other lisps?) to name functions ->foo (with the convention implying "convert anything to foo") or -bar (usually implying private) or *baz* (usually implying dynamic.)

@leoliu
Copy link
Owner

leoliu commented Sep 18, 2017

Looks like we need the following patch:

diff --git a/ggtags.el b/ggtags.el
index b3ab1f61..c0a9a9ee 100644
--- a/ggtags.el
+++ b/ggtags.el
@@ -1022,7 +1022,7 @@ (defun ggtags-find-tag-dwim (name &optional what)
                 (funcall (if (ggtags-sort-by-nearness-p)
                              #'file-relative-name #'ggtags-project-relative-file)
                          buffer-file-name)))
-       (shell-quote-argument name)))))
+       "--" (shell-quote-argument name)))))
 
 (defun ggtags-find-tag-mouse (event)
   (interactive "e")
@@ -1034,7 +1034,7 @@ (defun ggtags-find-tag-mouse (event)
 ;; Another option for `M-.'.
 (defun ggtags-find-definition (name)
   (interactive (list (ggtags-read-tag 'definition current-prefix-arg)))
-  (ggtags-find-tag 'definition (shell-quote-argument name)))
+  (ggtags-find-tag 'definition "--" (shell-quote-argument name)))
 
 (defun ggtags-setup-libpath-search (type name)
   (pcase (and ggtags-global-search-libpath-for-reference
@@ -1056,13 +1056,13 @@ (defun ggtags-setup-libpath-search (type name)
 (defun ggtags-find-reference (name)
   (interactive (list (ggtags-read-tag 'reference current-prefix-arg)))
   (ggtags-setup-libpath-search 'reference name)
-  (ggtags-find-tag 'reference (shell-quote-argument name)))
+  (ggtags-find-tag 'reference "--" (shell-quote-argument name)))
 
 (defun ggtags-find-other-symbol (name)
   "Find tag NAME that is a reference without a definition."
   (interactive (list (ggtags-read-tag 'symbol current-prefix-arg)))
   (ggtags-setup-libpath-search 'symbol name)
-  (ggtags-find-tag 'symbol (shell-quote-argument name)))
+  (ggtags-find-tag 'symbol "--" (shell-quote-argument name)))
 
 (defun ggtags-quote-pattern (pattern)
   (prin1-to-string (substring-no-properties pattern)))

@jonathanj
Copy link
Author

That looks like it'll work. Thanks for the lightning quick response!

@leoliu leoliu closed this as completed in 6293c43 Sep 19, 2017
@leoliu
Copy link
Owner

leoliu commented Sep 19, 2017

Thanks.

@jonathanj
Copy link
Author

Hmm, something I actually forgot to consider was my secondary issue with \*foo\*. The issue here is that this is an invalid regular expression not that it's being accidentally parsed as an option. Should I file another issue for this?

@leoliu
Copy link
Owner

leoliu commented Sep 19, 2017

Do you have a proposal how to fix the second issue? I experimented with it and found that it seemed to treat any string with some chars (e.g. *) in it as regexp.

@jonathanj
Copy link
Author

There is a --literal option to GNU Global that treats the search term as literal text instead of a regular expression. I think for the case where the symbol name is identified from the point it would make sense to pass --literal; however when the user is prompted for a symbol name it may be expected that a regular expression can be given, at least that's the current behaviour and it may be undesirable to change that.

I would be happy with an option to turn --literal on and off globally.

@leoliu
Copy link
Owner

leoliu commented Sep 19, 2017

I did note --literal but it seemed to do something else. Could you try also to see if it does what you need?

@jonathanj
Copy link
Author

I tried it before writing my comment and it produced the expected result, namely global --literal -sx \*foo\* produced a result for my variable named *foo* instead of an error about an invalid regular expression. I also tried some names without special characters in them and they still work.

Did you see different behaviour?

@leoliu leoliu reopened this Sep 19, 2017
@leoliu
Copy link
Owner

leoliu commented Sep 19, 2017

I did see a different result. Will look into it in more details tomorrow. Thanks.

@leoliu
Copy link
Owner

leoliu commented Sep 20, 2017

--literal makes no difference in the following example:

global --literal -v --result=grep --color=always --path-style=shorter --from-here=77:emulator/beam/erl_process.c -- \*ok\*

global: regular expression is not allowed with the --from-here option.

@jonathanj
Copy link
Author

Reading the source code for GNU Global, this looks like a bug to me. --literal should imply that there is in fact no regular expression.

However it's worth noting that adding --literal is not a regression here because at the moment trying to find a symbol named *ok* will fail in exactly the same way. In the worst case the behaviour remains the same, in the best case GNU Global fixes the interaction between --literal and --from-here and it works as intended with the --literal option.

I'll send a mail to the GNU Global bug report list.

@jonathanj
Copy link
Author

@leoliu I reported the bug to the GNU Global mailing list with a patch, which was accepted: http://lists.gnu.org/archive/html/global-commit/2017-09/msg00003.html

I don't think this has made it into a stable release yet though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants