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

fix wrong "text" parsing #5838

Closed
wants to merge 1 commit into from
Closed

fix wrong "text" parsing #5838

wants to merge 1 commit into from

Conversation

skiner68
Copy link

@skiner68 skiner68 commented Jul 3, 2017

accept lines containing dots only - I thing check position index should be $position, not $pos

accept lines containing dots only - I thing check position index should be $position, not $pos
@alecpl
Copy link
Member

alecpl commented Jul 3, 2017

Looks like you might be right, but do you have a sample sieve script that fails to parse correctly? I'd like to add a test for this.

@alecpl
Copy link
Member

alecpl commented Jul 3, 2017

Nevermind, I fixed this in 8b61d6a.

@alecpl alecpl closed this Jul 3, 2017
@skiner68
Copy link
Author

skiner68 commented Jul 3, 2017

Great, you were fast. Just one thing - should not $position be incremented once more in case line ends with "\r\n"? Honestly I have not checked this practically, but length of "\r\n" is 2 symbols, not 1 symbol like in case of simple "\n", so the $position value may be wrong in this case. Please see my original pull request.

@alecpl
Copy link
Member

alecpl commented Jul 3, 2017

You're right. I couldn't find a sample that breaks without this, but I fixed it anyway.

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

Successfully merging this pull request may close these issues.

2 participants