Re: libidn2 support
Tim Ruehsen writes: > Added a branch 'coverage' based on branch 'libunistring'. > > It contains a make target 'check-coverage' which runs the test suite with > collection coverage stats and generating HTML output in directory 'lcov'. > > It adds a NFC quick check to avoid unneeded normalizations (explained in > http://unicode.org/reports/tr15/#Detecting_Normalization_Forms). > > I also added a few tests to cover more code paths than before, raising > coverage to: > lines..: 88.8% (656 of 739 lines) > functions..: 95.1% (39 of 41 functions) > > That was the low hanging fruits, covering more code paths is beyond my time. > You could remove the case_flags from punycode_encode() - not used in libidn2. Neat. Take a look in cfg.mk: as part of the release process, coverage reports are generated too. It looks similar to your rules, not sure if there is any difference. They are online here: https://www.gnu.org/software/libidn/libidn2/coverage/libidn2/index.html > Interestingly, ltr_ends_ok() in bidi.c is never called. That means > ltr_lenescsetonbnnsm_ok() always returns false. Not sure what is going on > here. Maybe just missing test cases - they added more cases for Unicode > > 6.3.0. Interesting -- this may indicate a bug in the implementation, worth looking into comparing with the RFC. Or an RFC bug :-) /Simon signature.asc Description: PGP signature ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
On Wednesday, December 7, 2016 8:59:39 AM CET Simon Josefsson wrote: > Den Tue, 06 Dec 2016 17:03:04 +0100 > > skrev Re: libidn2 support: > > On Monday, December 5, 2016 10:00:32 AM CET Simon Josefsson wrote: > > > Hi again. I have added you now. There is no real work going on > > > with libidn2, but Hanno Böck said he may have found more > > > security vulnerabilities, so it would be nice to be able to do a > > > quick security release if needed. Therefor, it appears preferrable > > > to push your stuff to a branch meanwhile. I'm happy to review when > > > it is on a branch, and hopefully we can make test releases from the > > > branch too. > > > > Hi Simon, > > > > just put my stuff into 4 different branches within your Gitlab repo. > > Hi Tim. Yay! > > > Please review/merge in this order: > Very good to split things up, thank you. Let's try to do low-hanging > fruit one at a time. > > > # branch 'fixes' > > - fix two crashes in lookup and register functions > > - avoid tainting insertname/lookupname on error > > Can you write self-tests that trigger these issues? That makes it much > easier to evaluate the patches. > > > - use binary search instead of linear search in idna table > > How much do the table grow by adding UNASSIGNED code points to the > library size? I like the patch in general, but I am concerned that it > adds a lot of static size to the library. Is having the UNASSIGNED > code points in the idna_table array really necessary? It seems your > search function results UNASSIGNED if result==NULL anyway? I don't > recall if there is any semantic difference between a code point that is > UNASSIGNED and a code point that does not have any property at all. I chose a different table setup (pushed -f to 'tr46') which reduces static data size from 750k to ~35k. So together with removing the UNASSIGNED entries from the IDNA table (data.c), the library size reduces from 223352 (Debian libidn2 0.11) to 195952, both stripped :-) Regards, Tim signature.asc Description: This is a digitally signed message part. ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
On Wednesday, December 7, 2016 8:59:39 AM CET Simon Josefsson wrote: > Den Tue, 06 Dec 2016 17:03:04 +0100 > > skrev Re: libidn2 support: > > On Monday, December 5, 2016 10:00:32 AM CET Simon Josefsson wrote: > > > Hi again. I have added you now. There is no real work going on > > > with libidn2, but Hanno Böck said he may have found more > > > security vulnerabilities, so it would be nice to be able to do a > > > quick security release if needed. Therefor, it appears preferrable > > > to push your stuff to a branch meanwhile. I'm happy to review when > > > it is on a branch, and hopefully we can make test releases from the > > > branch too. > > > > Hi Simon, > > > > just put my stuff into 4 different branches within your Gitlab repo. > > Hi Tim. Yay! > > > Please review/merge in this order: > Very good to split things up, thank you. Let's try to do low-hanging > fruit one at a time. > > > # branch 'fixes' > > - fix two crashes in lookup and register functions > > - avoid tainting insertname/lookupname on error > > Can you write self-tests that trigger these issues? That makes it much > easier to evaluate the patches. Added a branch 'coverage' based on branch 'libunistring'. It contains a make target 'check-coverage' which runs the test suite with collection coverage stats and generating HTML output in directory 'lcov'. It adds a NFC quick check to avoid unneeded normalizations (explained in http://unicode.org/reports/tr15/#Detecting_Normalization_Forms). I also added a few tests to cover more code paths than before, raising coverage to: lines..: 88.8% (656 of 739 lines) functions..: 95.1% (39 of 41 functions) That was the low hanging fruits, covering more code paths is beyond my time. You could remove the case_flags from punycode_encode() - not used in libidn2. Interestingly, ltr_ends_ok() in bidi.c is never called. That means ltr_lenescsetonbnnsm_ok() always returns false. Not sure what is going on here. Maybe just missing test cases - they added more cases for Unicode > 6.3.0. Regards, Tim signature.asc Description: This is a digitally signed message part. ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
On Wednesday, December 7, 2016 8:59:39 AM CET Simon Josefsson wrote: > Den Tue, 06 Dec 2016 17:03:04 +0100 > > skrev Re: libidn2 support: > > On Monday, December 5, 2016 10:00:32 AM CET Simon Josefsson wrote: > > > Hi again. I have added you now. There is no real work going on > > > with libidn2, but Hanno Böck said he may have found more > > > security vulnerabilities, so it would be nice to be able to do a > > > quick security release if needed. Therefor, it appears preferrable > > > to push your stuff to a branch meanwhile. I'm happy to review when > > > it is on a branch, and hopefully we can make test releases from the > > > branch too. > > > > Hi Simon, > > > > just put my stuff into 4 different branches within your Gitlab repo. Watch out, I added stuff to branch 'tr46' and rebased 'libunistring' on 'tr46'. You likely need a git fetch -f. Tim signature.asc Description: This is a digitally signed message part. ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
On Wednesday, December 7, 2016 8:59:39 AM CET Simon Josefsson wrote: > Den Tue, 06 Dec 2016 17:03:04 +0100 > > skrev Re: libidn2 support: > > On Monday, December 5, 2016 10:00:32 AM CET Simon Josefsson wrote: > > > Hi again. I have added you now. There is no real work going on > > > with libidn2, but Hanno Böck said he may have found more > > > security vulnerabilities, so it would be nice to be able to do a > > > quick security release if needed. Therefor, it appears preferrable > > > to push your stuff to a branch meanwhile. I'm happy to review when > > > it is on a branch, and hopefully we can make test releases from the > > > branch too. > > > > Hi Simon, > > > > just put my stuff into 4 different branches within your Gitlab repo. > > Hi Tim. Yay! > > > Please review/merge in this order: > Very good to split things up, thank you. Let's try to do low-hanging > fruit one at a time. > > > # branch 'fixes' > > - fix two crashes in lookup and register functions > > - avoid tainting insertname/lookupname on error > > Can you write self-tests that trigger these issues? That makes it much > easier to evaluate the patches. The crashes are absolutely obvious: Unchecked call to u8_strconv_from_locale(). Crashes on src==NULL. A test for for the tainting issue needs to cover each single error condition that can happen. No covering all possibilities is half-baken. Let's address such things in a future work-to-do, namely 'test code coverage'. I would suggest to add TravisCI (or whatever CI you prefer) and Coveralls.io hooks. With that, it is much more straight forward to write exactly that test code that is needed. I did so in other projects. Perhaps I find time to open up another branch. > > - use binary search instead of linear search in idna table > > How much do the table grow by adding UNASSIGNED code points to the > library size? I like the patch in general, but I am concerned that it > adds a lot of static size to the library. Is having the UNASSIGNED > code points in the idna_table array really necessary? It seems your > search function results UNASSIGNED if result==NULL anyway? > I don't > recall if there is any semantic difference between a code point that is > UNASSIGNED and a code point that does not have any property at all. I am puzzled... you always added UNASSIGNED to the static array. My patch removes it and saves space :-). That is why I don't like Perl scripts - the original authors can't read them any more after a while :-) Please don't take that too serious, just kidding a bit. Number of entries omitted by my patch: $ wget ftp://ftp.iana.org/assignments/idna-tables-5.2.0/idna-tables-5.2.0.txt $ grep -c UNASSIGNED idna-tables-5.2.0.txt 494 $ wget ftp://ftp.iana.org/assignments/idna-tables-6.3.0/idna-tables-6.3.0.txt $ grep -c UNASSIGNED idna-tables-6.2.0.txt 548 Tim signature.asc Description: This is a digitally signed message part. ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
Den Tue, 06 Dec 2016 17:03:04 +0100 skrev Re: libidn2 support: > On Monday, December 5, 2016 10:00:32 AM CET Simon Josefsson wrote: > > Hi again. I have added you now. There is no real work going on > > with libidn2, but Hanno Böck said he may have found more > > security vulnerabilities, so it would be nice to be able to do a > > quick security release if needed. Therefor, it appears preferrable > > to push your stuff to a branch meanwhile. I'm happy to review when > > it is on a branch, and hopefully we can make test releases from the > > branch too. > > Hi Simon, > > just put my stuff into 4 different branches within your Gitlab repo. Hi Tim. Yay! > Please review/merge in this order: Very good to split things up, thank you. Let's try to do low-hanging fruit one at a time. > # branch 'fixes' > - fix two crashes in lookup and register functions > - avoid tainting insertname/lookupname on error Can you write self-tests that trigger these issues? That makes it much easier to evaluate the patches. > - use binary search instead of linear search in idna table How much do the table grow by adding UNASSIGNED code points to the library size? I like the patch in general, but I am concerned that it adds a lot of static size to the library. Is having the UNASSIGNED code points in the idna_table array really necessary? It seems your search function results UNASSIGNED if result==NULL anyway? I don't recall if there is any semantic difference between a code point that is UNASSIGNED and a code point that does not have any property at all. /Simon pgp96d5V6Cz83.pgp Description: OpenPGP digital signatur ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
On Monday, December 5, 2016 10:00:32 AM CET Simon Josefsson wrote: > Hi again. I have added you now. There is no real work going on with > libidn2, but Hanno Böck said he may have found more > security vulnerabilities, so it would be nice to be able to do a quick > security release if needed. Therefor, it appears preferrable to push > your stuff to a branch meanwhile. I'm happy to review when it is on a > branch, and hopefully we can make test releases from the branch too. Hi Simon, just put my stuff into 4 different branches within your Gitlab repo. Please review/merge in this order: # branch 'fixes' - fix two crashes in lookup and register functions - avoid tainting insertname/lookupname on error - use binary search instead of linear search in idna table # branch 'unicode6.3.0' (based on 'fixes') Basically does what it's name says, moves on to Unicode 6.3.0 from currently 5.2.0. Also does a few cleanups in the build area (Makefiles, URLs). # branch 'tr46' (based on 'unicode 6.3.0') Does what it says. Adds more error codes, flags to the API and options to idn2. Also uses the official test suite / test cases from Unicode. # branch 'libunicode' (based on 'unicode6.3.0) This single commit introduces dynamic usage of gnulib, introduces ./bootstrap as proper bootstrapping mechanism, removes all the gl and src/gl stuff from the repo. It links with libunistring with fallback to gnulib for functions that do not exist (e.g. *joining*()). Currently, the tests fail on Debian unstable since there is an buggy libunistring0 0.9.3. Upgrading to libunistring2 (0.9.6) from experimental does it for me. I am not sure how the status is with other distributions. I am sure I missed to remove some autogenerated files from the repo and other little things... so please (everybody here) test, review, comment, ... Regards, Tim signature.asc Description: This is a digitally signed message part. ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
Hi again. I have added you now. There is no real work going on with libidn2, but Hanno Böck said he may have found more security vulnerabilities, so it would be nice to be able to do a quick security release if needed. Therefor, it appears preferrable to push your stuff to a branch meanwhile. I'm happy to review when it is on a branch, and hopefully we can make test releases from the branch too. /Simon > Hi Simon, > > thanks for your offer ! > > I would be happy to push my changes. > There is still time to test/discuss until the next release, I guess. > Are you ok, if I push to master or would you like to have a separate > branch ? > > My name on Gitlab (and elsewhere) is rockdaboot. > > Regards, Tim > > > On Thursday, December 1, 2016 2:39:55 PM CET Simon Josefsson wrote: > > Hi Tim. Thanks, this is really cool. Do you want commit access to > > libidn2 so you can push this? What's your gitlab username? I have > > limited time and don't want to be in your way for this to happen. > > I'm happy to do review, but can't promise when. > > > > /Simon > > > > tis 2016-11-29 klockan 22:23 +0100 skrev Tim Rühsen: > > > On Mittwoch, 2. November 2016 10:04:19 CET Daniel Stenberg wrote: > > > > On Wed, 2 Nov 2016, Simon Josefsson wrote: > > > > > Switching from IDNA2003 to IDNA2008 requires thought. They > > > > > are not compatible, and IDNA2008 requires pre-processing > > > > > (e.g., UTS #46 [1]) to > > > > > be usable in practice. Libidn2 does not implement any > > > > > pre-processing, it is a pure IDNA2008 implementation. > > > > > > > > > > [1] http://www.unicode.org/reports/tr46/ > > > > > > > > Thanks for this Simon. I wasn't aware. > > > > > > > > Based on this, I suppose my recommendation is that people > > > > simply switch off > > > > IDN support in curl builds until further notice to stay safe. > > > > The old way > > > > was open for trickery and the new way is incomplete. > > > > > > Please review and comment > > > https://gitlab.com/rockdaboot/libidn2/tree/tr46 (It works for me > > > ). > > > > > > I would like to open a pull request to libidn2 soon. > > > That code will be useful for any DNS clients resolving IDNs. > > > > > > From my mail to help-libidn mailing list: > > > > > > The code introduces new flags for the lookup functions > > > IDN2_TRANSITIONAL and IDN2_NONTRANSITIONAL (mutual exclusive). > > > > > > IDN2_TRANSITIONAL enables the TR46 transitional process. > > > IDN2_NONTRANSITIONAL enables the TR46 non-transitional process. > > > > > > Corresponding, the 'idn2' utility now has -T/--tr46t to enable > > > transitional > > > lookup and -N/--tr46nt to enable non-transitional lookup. > > > > > > Example: > > > $ src/idn2 -T faß.de > > > fass.de > > > > > > $ src/idn2 -N faß.de > > > xn--fa-hia.de > > > > > > To show the TR46 mapping (upper->lowercase) in work: > > > $ src/idn2 Faß.de > > > idn2: lookup: string contains a disallowed character > > > > > > $ src/idn2 -T Faß.de > > > fass.de > > > > > > $ src/idn2 -N Faß.de > > > xn--fa-hia.de > > > > > > Regards, Tim > pgpLHkjFc_PCp.pgp Description: OpenPGP digital signatur ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
Hi Simon, thanks for your offer ! I would be happy to push my changes. There is still time to test/discuss until the next release, I guess. Are you ok, if I push to master or would you like to have a separate branch ? My name on Gitlab (and elsewhere) is rockdaboot. Regards, Tim On Thursday, December 1, 2016 2:39:55 PM CET Simon Josefsson wrote: > Hi Tim. Thanks, this is really cool. Do you want commit access to > libidn2 so you can push this? What's your gitlab username? I have > limited time and don't want to be in your way for this to happen. I'm > happy to do review, but can't promise when. > > /Simon > > tis 2016-11-29 klockan 22:23 +0100 skrev Tim Rühsen: > > On Mittwoch, 2. November 2016 10:04:19 CET Daniel Stenberg wrote: > > > On Wed, 2 Nov 2016, Simon Josefsson wrote: > > > > Switching from IDNA2003 to IDNA2008 requires thought. They are not > > > > compatible, and IDNA2008 requires pre-processing (e.g., UTS #46 [1]) > > > > to > > > > be usable in practice. Libidn2 does not implement any pre-processing, > > > > it is a pure IDNA2008 implementation. > > > > > > > > [1] http://www.unicode.org/reports/tr46/ > > > > > > Thanks for this Simon. I wasn't aware. > > > > > > Based on this, I suppose my recommendation is that people simply switch > > > off > > > IDN support in curl builds until further notice to stay safe. The old > > > way > > > was open for trickery and the new way is incomplete. > > > > Please review and comment https://gitlab.com/rockdaboot/libidn2/tree/tr46 > > (It works for me ). > > > > I would like to open a pull request to libidn2 soon. > > That code will be useful for any DNS clients resolving IDNs. > > > > From my mail to help-libidn mailing list: > > > > The code introduces new flags for the lookup functions IDN2_TRANSITIONAL > > and IDN2_NONTRANSITIONAL (mutual exclusive). > > > > IDN2_TRANSITIONAL enables the TR46 transitional process. > > IDN2_NONTRANSITIONAL enables the TR46 non-transitional process. > > > > Corresponding, the 'idn2' utility now has -T/--tr46t to enable > > transitional > > lookup and -N/--tr46nt to enable non-transitional lookup. > > > > Example: > > $ src/idn2 -T faß.de > > fass.de > > > > $ src/idn2 -N faß.de > > xn--fa-hia.de > > > > To show the TR46 mapping (upper->lowercase) in work: > > $ src/idn2 Faß.de > > idn2: lookup: string contains a disallowed character > > > > $ src/idn2 -T Faß.de > > fass.de > > > > $ src/idn2 -N Faß.de > > xn--fa-hia.de > > > > Regards, Tim signature.asc Description: This is a digitally signed message part. ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
Re: libidn2 support
Hi Tim. Thanks, this is really cool. Do you want commit access to libidn2 so you can push this? What's your gitlab username? I have limited time and don't want to be in your way for this to happen. I'm happy to do review, but can't promise when. /Simon tis 2016-11-29 klockan 22:23 +0100 skrev Tim Rühsen: > On Mittwoch, 2. November 2016 10:04:19 CET Daniel Stenberg wrote: > > On Wed, 2 Nov 2016, Simon Josefsson wrote: > > > Switching from IDNA2003 to IDNA2008 requires thought. They are not > > > compatible, and IDNA2008 requires pre-processing (e.g., UTS #46 [1]) to > > > be usable in practice. Libidn2 does not implement any pre-processing, > > > it is a pure IDNA2008 implementation. > > > > > > [1] http://www.unicode.org/reports/tr46/ > > > > Thanks for this Simon. I wasn't aware. > > > > Based on this, I suppose my recommendation is that people simply switch off > > IDN support in curl builds until further notice to stay safe. The old way > > was open for trickery and the new way is incomplete. > > Please review and comment https://gitlab.com/rockdaboot/libidn2/tree/tr46 > (It works for me ). > > I would like to open a pull request to libidn2 soon. > That code will be useful for any DNS clients resolving IDNs. > > From my mail to help-libidn mailing list: > > The code introduces new flags for the lookup functions IDN2_TRANSITIONAL and > IDN2_NONTRANSITIONAL (mutual exclusive). > > IDN2_TRANSITIONAL enables the TR46 transitional process. > IDN2_NONTRANSITIONAL enables the TR46 non-transitional process. > > Corresponding, the 'idn2' utility now has -T/--tr46t to enable transitional > lookup and -N/--tr46nt to enable non-transitional lookup. > > Example: > $ src/idn2 -T faß.de > fass.de > > $ src/idn2 -N faß.de > xn--fa-hia.de > > To show the TR46 mapping (upper->lowercase) in work: > $ src/idn2 Faß.de > idn2: lookup: string contains a disallowed character > > $ src/idn2 -T Faß.de > fass.de > > $ src/idn2 -N Faß.de > xn--fa-hia.de > > Regards, Tim signature.asc Description: This is a digitally signed message part ___ Help-libidn mailing list Help-libidn@gnu.org https://lists.gnu.org/mailman/listinfo/help-libidn
