Use ordinal comparisons when parsing IRC messages. (Unicode bugfix)

meebey / SmartIrc4net

IRC C# Library

http://www.meebey.net/projects/smartirc4net/

Other

127 stars 52 forks source link

Use ordinal comparisons when parsing IRC messages. (Unicode bugfix) #12

Closed DeathByNukes closed 9 years ago

DeathByNukes commented 11 years ago

Fixes a bug where messages starting with "combining" Unicode characters cause the parser to fail. Likely improves performance too.

meebey commented 11 years ago

Can you give an example how this would screw up the parser. I can"t come up with something...

DeathByNukes commented 11 years ago

When IrcConnection.Encoding == Encoding.UTF8 and someone sends a message starting with unicode character \u0307 (alt+775 in mIRC), e.Data.Message in the corresponding OnChannelMessage event will be null.

ttjkfwd Notice the mark above the colon in this image.

meebey commented 11 years ago

I see what the parser did in your screenshot but I am unable to reproduce this myself. Maybe Mono behaves differently in this regard...

DeathByNukes commented 10 years ago

Maybe so...

DeathByNukes commented 10 years ago

Either way, I don't see a reason to reject this change. I assure you it works; I've been running this version almost a year now with no problems. Ordinal comparisons are more efficient and I assume they are truer to the IRC standard. (they compare the bytes directly with no fancy logic)

meebey commented 10 years ago

I am not merging arbitrary changes for unreproducible issues. I need at least a test-case that fails on Mono or .NET while it should work (like your screenshot suggests). If this matter is true, then ALL uses of IndexOf and possibly other string methods need to be changed for protocol message parsing.

DeathByNukes commented 10 years ago

Unreproducible? It sounded like you only attempted to reproduce it in Mono then gave up. Do you need more information?

meebey commented 9 years ago

Yes exactly, I don't have .NET anywhere handy :) I will merge this fix anyhow since your screenshot clearly shows the bug. Thanks for finding and fixing this!