MONTHDAY must not require a leading zero for day-of-month < 10 #5
Conversation
| @@ -48,7 +48,7 @@ URI %{URIPROTO}://(?:%{USER}(?::[^@]*)?@)?(?:%{URIHOST})?(?:%{URIPATHPARAM})? | |||
| MONTH \b(?:Jan(?:uary)?|Feb(?:ruary)?|Mar(?:ch)?|Apr(?:il)?|May|Jun(?:e)?|Jul(?:y)?|Aug(?:ust)?|Sep(?:tember)?|Oct(?:ober)?|Nov(?:ember)?|Dec(?:ember)?)\b | |||
| MONTHNUM (?:0?[1-9]|1[0-2]) | |||
| MONTHNUM2 (?:0[1-9]|1[0-2]) | |||
| MONTHDAY (?:(?:0[1-9])|(?:[12][0-9])|(?:3[01])|[1-9]) | |||
| MONTHDAY (?:(?:(0|\s)?[1-9])|(?:[12][0-9])|(?:3[01])|[1-9]) | |||
There was a problem hiding this comment.
So. of if you breakdown the original regexp by brancheS:
- (?:0[1-9])
- (?:[12][0-9])
- (?:3[01])
- [1-9]
This should already match single-digit month numbers.
Further, this patch makes a space part of the MONTHDAY match, which is not, in my opinion, the correct behavior.
There was a problem hiding this comment.
This should already match single-digit month numbers.
Specifically, when I read this original regexp, I believe it would match single-digit numbers. If it doesn't, this is obivously a bug as you have reported, but I want to make sure we fix the bug which is located at the end of the old regexp, not the beginning.
Hmm...
There was a problem hiding this comment.
Also, I agree that the whitespace match probably should not be part of the MONTHDAY match.
On a side note, I think that the original:
(?:(?:0[1-9])|(?:[12][0-9])|(?:3[01])|[1-9])
is equivalent to the following:
(?:(?:0?[1-9])|(?:[12][0-9])|(?:3[01]))
I don't know if the shorter one is an improvement though.
|
Closing this PR becuase of long staled pending CLA, opening #84 If the CLA is signed, more than welcome to reopen the PR and get this change in. |
Migrated from: elastic/logstash#1353
More about the problem on: https://logstash.jira.com/browse/LOGSTASH-2199