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

Escaped braces aren't handled properly #56

Closed
micimize opened this issue May 11, 2021 · 6 comments
Closed

Escaped braces aren't handled properly #56

micimize opened this issue May 11, 2021 · 6 comments
Assignees

Comments

@micimize
Copy link

It seems that \{ and \} are not handled properly in both pattern patching and path evaluation:

  • The pattern **/log file \{????-??-??\}.txt will fail with a syntax error
  • The input path src/logs/log file {2021-05-11}.txt will fail with any pattern (this is a valid path)

If I remove both from my test the other patterns and path combinations work fine. This does not seem to be the case for brackets, e.g. src/logs/log file [2021-05-11].txt
and **/log file [????-??-??].txt. I am using doublestar.Match.

@bmatcuk bmatcuk self-assigned this May 12, 2021
@bmatcuk
Copy link
Owner

bmatcuk commented May 12, 2021

Hmm, are you using doublestar v4? What OS are you on? How are you building your pattern? You'll need to double the backslashes to get them to stay in the string, ie "**/log file \\{????-??-??\\}.txt".

I cannot reproduce this issue using v4 on macOS.

@micimize
Copy link
Author

micimize commented May 12, 2021

I think I was maybe mistaken as to the exact issue – there is an issue with both \[ and \{ escaping.
If there is a match, they succeed, but if not, they return a syntax error. See https://play.golang.org/p/Saso864XooB

copied for reference
package main

import (
	"fmt"

	"github.com/bmatcuk/doublestar/v4"
)

func main() {
	patterns := []string{
		//`\{????-??-??\}.txt`,
		//`\[????-??-??\].txt`,
		`\{*\}.txt`,
		`\[*\].txt`,
	}
	names := []string{
		`{2021-05-01}.txt`,
		`[2021-05-01].txt`,
		`normal text`,
	}
	for _, p := range patterns {
		for _, n := range names {
			match, err := doublestar.Match(p, n)
			fmt.Println(p, n, match, err)
		}
	}

}

/* output
\{*\}.txt {2021-05-01}.txt true <nil>
\{*\}.txt [2021-05-01].txt false syntax error in pattern
\{*\}.txt normal text false syntax error in pattern
\[*\].txt {2021-05-01}.txt false syntax error in pattern
\[*\].txt [2021-05-01].txt true <nil>
\[*\].txt normal text false syntax error in pattern
*/

@micimize
Copy link
Author

micimize commented Jun 7, 2021

@bmatcuk I found the issue – every iteration of the loop consuming pattern validates the remaining pattern:

doublestar/match.go

Lines 279 to 281 in 1fd2e2d

if validate && patIdx < patLen && !doValidatePattern(pattern[patIdx:], separator) {
return false, ErrBadPattern
}

The default fallthrough after '\\' is matched breaks before bumping patIdx when a match is rejected, resulting in an invalid pattern at validation time:

doublestar/match.go

Lines 241 to 245 in 1fd2e2d

if patRune != nameRune {
break
}
patIdx += patRuneLen

Why validate the pattern every loop? doValidatePattern is called nameLen * patLen times this way

@bmatcuk
Copy link
Owner

bmatcuk commented Jun 12, 2021

Hey @micimize! I'm so sorry I didn't see your reply back in May! I never got a notification from github that you replied! Your test cases were most helpful and I was able to fix the bug fairly quickly. I only wish I would have seen your reply a month ago so I could have fixed it then 😢

Anyway, I just released v4.0.2 with the fix. Let me know if you have any trouble with it.

By the way, doValidatePattern is only called if all else fails. Everything in the loop above it will either return or continue the loop before reaching that point. So, it will only reach that if the pattern has been valid up until now, but a match could not be made, and neither ** or * backtracking is possible. At that point, we already know we're returning false, we just want to know if we're also returning an error.

@bmatcuk
Copy link
Owner

bmatcuk commented Aug 25, 2021

I'm going to close this issue since there hasn't been any activity. Let me know if you run into trouble with the fix!

@bmatcuk bmatcuk closed this as completed Aug 25, 2021
@micimize
Copy link
Author

will do! Sorry I haven't gotten around to actually testing it but will ping here if there are any issues when I do

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