Marusyk / grok.net

.NET implementation of the grok 📝
MIT License
290 stars 55 forks source link

Invalid custom patterns are silently ignored #52

Closed sandreas closed 2 years ago

sandreas commented 2 years ago

Hello,

at first, I would like to thank you for this awesome project. Very helpful! During my tests I noticed something, that might be a little confusing or at least there is room for improvement...

I used the following code with a custom pattern:

var stream = new MemoryStream(Encoding.Default.GetBytes("NOTDIRSEP [^/\\]*"));
var grok = new Grok("input/%{NOTDIRSEP:genre}/", stream);
var result = grok.Parse("input/Fantasy/");

The result was empty, no errors. After some debugging I noticed, that I accidentaly used an invalid pattern (escaping is hard! ;-) ):

# invalid pattern
NOTDIRSEP [^/\\]*

# valid pattern
NOTDIRSEP [^/\\\\]*

So, this produced the expected result:

var stream = new MemoryStream(Encoding.Default.GetBytes("NOTDIRSEP [^/\\\\]*"));
var grok = new Grok("input/%{NOTDIRSEP:genre}/", stream);
var result = grok.Parse("input/Fantasy/");

It turned out, that the invalid pattern was just silently ignored without the possibility of handling an error - except running the ignored regex check manually beforehand. See https://github.com/Marusyk/grok.net/blob/8130f2fc5d05d4bf50c911bf1dd971d648dffa38/src/Grok.Net/Grok.cs#L161

There is also a little inconsistency:

I think there should be at least something like public List<(string, string)> InvalidCustomPatterns, while adding a list item (customPattern, errorMessage), if the Regex-Check fails - or, since there already is an exception, just rethrowing it with a more detailed error.

What do you think?

Marusyk commented 2 years ago

Hi @sandreas Thank you very much for your interest in the project and your ideas on improving that. I totally agree, we definitely should fix that. I don't have much time now, would you like to submit a PR with fixes?

sandreas commented 2 years ago

I don't have much time now

I just saw that you are from Ukraine. Let me express my support and sympathy for you and your people, I hope you all get out of this horror as soon as possible.

would you like to submit a PR with fixes?

I'll try :-)

Marusyk commented 2 years ago

Yes, I'm from Ukraine. My country has been invaded by Russia and is currently at war Thank you so much for your support ❤ I will try to review the PR as soon as I can