Re: Fix Win64 oddity in libidn 1.38 stringprep

[Simon Josefsson]

sön 2022-02-20 klockan 10:55 +0100 skrev Francesco Pretto:
> On Sun, 20 Feb 2022 at 08:34, Simon Josefsson <simon@josefsson.org>
> wrote:
> > 
> > Francesco Pretto <ceztko@gmail.com> writes:
> > Thanks.  What error message do you get, or how did you notice this?
> 
> I noticed trying to call stringprep this way:
> 
> int rc = stringprep_profile(password.data(), &password_sasl,
> "SASLprep", STRINGPREP_NO_UNASSIGNED);
> 
>  Any string resulted in being treated as empty string.

Thanks.

>  > We  do build and self-test Win64 via CI/CD so I'm surprised to see
> this.
> 
> This issue should trigger only with MSVC family of compilers. Do
> you test MinGW-w64 builds?

Yes, that is the intention at least:

https://gitlab.com/libidn/libidn/-/jobs/2114691787

Why does it happens only on MSVC?  If I understood correctly, 'long' is
32 bit on Win64, and should be with MSVC as well.

How did you build libidn using MSVC?  The windows/ project files?

> > although I'm not certain your
> > patch is the proper fix -- maybe 'glong' should be mapped to a
> > different
> > type instead.
> > 
> 
> I actually tried it: I tried to map it to both size_t or ssize_t
> first. This ended in either being wrong or the compiler was warning
> about comparison between signed and unsigned (MSVC compilers have
> this
> warning enabled by default). In the end, removing the glong type and
> name the types correctly, carefully choosing between size_t and
> ssize_t (or gsize_t/gssize_t), ended being the code that was more
> semantically correct, without producing warnings for comparisons
> between signed and unsigned. Maybe you can find a even better
> solution
> that I couldn't spot, but you may also end to my same conclusion. I
> understand the point of keeping such "g" prefixing types for syncing
> with external code, but it appears that the generic signed "long"
> type
> (with the expectation that it will scale on 64 bit archs) shouldn't
> be
> used at all in these small snippets: in all the cases it was
> semantically used as size like type for a buffer, or the length of a
> string, or a signed size/length where negative values are sentinels,
> hence the size_t and ssize_t types are more appropriate.

Yeah, I agree, this part of the code does not age well, but it has been
the same for 10+ years and used cross-platform including Win64.  Maybe
we could use libunistring for some Unicode operations instead, although
we have to be careful to not use it for NFKC since we need to preserve
the Unicode 3.2 NFKC behaviour for IDNA2003 Stringprep.

/Simon

signature.asc
Description: This is a digitally signed message part