[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: libidn2 support
From: |
Tim Ruehsen |
Subject: |
Re: libidn2 support |
Date: |
Wed, 07 Dec 2016 10:15:22 +0100 |
User-agent: |
KMail/5.2.3 (Linux/4.8.0-2-amd64; KDE/5.28.0; x86_64; ; ) |
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.
- Re: libidn2 support, Simon Josefsson, 2016/12/01
- Re: libidn2 support, Tim Ruehsen, 2016/12/01
- Re: libidn2 support, Simon Josefsson, 2016/12/05
- Re: libidn2 support, Tim Ruehsen, 2016/12/06
- Re: libidn2 support, Simon Josefsson, 2016/12/07
- Re: libidn2 support,
Tim Ruehsen <=
- Re: libidn2 support, Tim Ruehsen, 2016/12/07
- Re: libidn2 support, Tim Ruehsen, 2016/12/08
- Re: libidn2 support, Simon Josefsson, 2016/12/26
- Re: libidn2 support, Tim Ruehsen, 2016/12/09