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

Bad Escape Error parsing regex #2281

Closed
corylanou opened this issue Apr 14, 2015 · 5 comments
Closed

Bad Escape Error parsing regex #2281

corylanou opened this issue Apr 14, 2015 · 5 comments
Assignees
Milestone

Comments

@corylanou
Copy link
Contributor

> select * FROM /^payments\./
ERR: error parsing query: bad escape:  at line 1, char 26
@dgnorton
Copy link
Contributor

david [5:58 PM]
Regex syntax question for everyone. Currently, /foo\./ returns a Bad Escape error. I can fix that by either forcing the user to enter /foo\\./ or by disallowing / to ever appear in a regex. Thoughts? Alternative ideas? (edited)

otoolep [6:00 PM]
@david: I go with escaping. You never know what someone might want.

david [6:02 PM]
@otoolep: I should also say that all escapes have to be double escaped. e.g., if you wanted to find the literal text cpu.* you would have to enter /cpu\\.\\*/

david [6:04 PM]
@hottoddy: how common do you think it will be for users to need to match / in a regex?

david [6:05 PM]
e.g., do we have many users using measurement names or tag values like foo/bar?

david [6:10 PM]
If we disallow / in a regex, they could still use \x2F to match that char. http://play.golang.org/p/0OUeTVBsvC

/cc @benbjohnson

@beckettsean
Copy link
Contributor

\x2F for the edge case feels like an easier thing to maintain and document than double-escaping everything always. I vote we make the 80% case easy (especially since it's really more the 99% case.)

@pauldix
Copy link
Member

pauldix commented May 4, 2015

Why can't we escape / just like with other escapes? e.g. /foo\/bar/, /foo\\bar/, /foo\.bar/

Double escaping sounds very bad to me. I think it'll be a usability nightmare.

@dgnorton
Copy link
Contributor

dgnorton commented May 4, 2015

Because it's going through two parsers...InfluxQL and then Go's regex parser. Both use \ for escapes. In order to pass \ through to the regex parser, it currently has to be double escaped.

Since InfluxQL regexp uses / as the delimiter, we could disallow that char in regexes and then not check for escapes during InfluxQL parsing of regexes. The down side to this is that users would have to use \x2F to match / but I don't know how common that is.

@pauldix
Copy link
Member

pauldix commented May 4, 2015

For the InfluxQL parser, couldn't it just look at the next character after a / to see if it's a \? If it is, skip over that character and continue parsing. The InfluxQL parser should leave the escapes in so they can be passed on to the Go regex parser.

dgnorton added a commit that referenced this issue May 4, 2015
Make the InfluxQL parser check for '\/' escape and pass all other
escapes through to the Go regexp parser unmodified.
toddboom added a commit that referenced this issue May 6, 2015
fix #2281: passthru escapes when parsing regex
@dgnorton dgnorton removed the review label May 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants