Use ruby unicode normalize to avoid libidn C problems and heavy legacy ruby code

[jarthod]

This is a fix for #408 and the beginning of the simplification and modernization of the IDNA code. It does:

• Replaces libidn unicode normalize implementation by ruby's, to avoid the null terminated string problem and support more recent unicode versions.
• Replaces Pure unicode normalize by ruby's, to avoid hundreds of line of slow and legacy ruby code and support more recent unicode versions.

The result in terms of performance is the following (benchmark code commited, but needs to be run on master to measure the legacy "pure" code):

#              user     system      total        real
# pure     1.325948   0.000000   1.325948 (  1.326054)
# libidn   0.058067   0.000000   0.058067 (  0.058069)
# ruby     0.325062   0.000000   0.325062 (  0.325115)

The native ruby implementation is 5-6 times slower than libidn, but also 5 times faster than the existing Pure one.

Considering this is just a small part and the rest of the ruby code is more than one order of magnitude slower (doing a full normalize in the simple.rb benchmark for example takes 8.6 seconds for the same 100k iterations on my machine), I think this is more than enough, no need to try to shave off microseconds in the unicode_normalize code if the rest is a lot of ruby.

And we benefit from the native ruby function which is going to be maintained and improved.
Specs have been added to cover the problematic \x00 case.

[jarthod]

Ok I've fixed some encoding issues with template on ruby 2.7- (passed with some ascii strings it was failing) and most hound warnings. I've left comments to explain the changes. All tests are passing. Let me know if you have any comment.

As another validation step, I'm gonna run this version in my product (https://updown.io) which converts 150k+ URl regularly to see if I spot anything.

[jarthod]

I've just ran some test on 137k updown.io URLs, comparing normalization with previous and new code:

Using this branch of addressable:

> urls = Check.only(:type, :url).map { Addressable::URI.parse("#{_1.type}://#{_1.url}") }; urls.size
137546
> File.open("urls_after.txt", "w") { |f| f.puts urls.map(&:normalize) }

Same thing with main

> File.open("urls_before.txt", "w") { |f| f.puts urls.map(&:normalize) }

Then comparing the result (obfuscating the URLs):

> diff urls_before.txt urls_after.txt 
70777,70778c70777,70778
< https://xxx.com/?q=(+v%20%CC%84%20+)
< https://xxx.com/?q=(+v%20%CC%84%20+)
---
> https://xxx.com/?q=(+v%C2%AF%C2%A0+)
> https://xxx.com/?q=(+v%C2%AF%C2%A0+)

Only two URL impacted, the difference is the same, I looked closer it's one of the difference between NFKC and NFC.
The new version %C2%AF%C2%A0 is correct because it preserve the unicode character, the older version %20%CC%84%20 is an incorrect URL normalization because it applied NFKC instead of NFC.

So it looks all good here, I added two more specs to make sure the path is NFC normalized but NOT NFKC.

One of the tests failed in my latest push but it's unrelated (it's a ruby download issue on the windows instance), I can't restart it but if you can it'll surely pass :)

[dentarg]

Looks good to me, though I'm no expert at all in this area! I trust our tests and your testing @jarthod :-) Thank you for this very clear pull request.

[dentarg]

I was thinking about a major version bump, but I guess this very much a bug fix? Unless there are some very strange edge cases we don't know about and people rely on.

[jarthod]

Looks good to me, though I'm no expert at all in this area! I trust our tests and your testing @jarthod :-) Thank you for this very clear pull request.

Thanks! @brasic maybe you also have an opinion or feedback on this PR?

I was thinking about a major version bump, but I guess this very much a bug fix? Unless there are some very strange edge cases we don't know about and people rely on.

I don't think this deserves a major bump, it's more of a bugfix indeed.
#491 and #247 definitely will though :)

[dentarg]

#491 and #247 definitely will though :)

Hehe, yes, was thinking that too :)

[sporkmonger]

Looks good to me!

[sporkmonger]

Sorry about being slow to respond! I was out on vacation last week and the laptop stayed home.

[jarthod]

@sporkmonger no problem, thank you!