View Issue Details

IDProjectCategoryView StatusLast Update
0005173GNUnetbuild processpublic2019-02-28 11:17
Reporternikita Assigned ToChristian Grothoff  
PrioritynormalSeveritytweakReproducibilityhave not tried
Status closedResolutionfixed 
Product VersionGit master 
Target Version0.11.0Fixed in Version0.11.0 
Summary0005173: idn2 support
DescriptionSo I've found this patch in the "wild" (twitter.com > github.com > gist). In the absence of any fully functional CI/CD at the moment, is it reasonable to bump the idn requirement up to idn2? Are there any potential issues on older systems or non-GNU systems?
https://gist.github.com/multiSnow/1be9d7345e042a77bb3c0e984f60e7b2
TagsNo tags attached.

Activities

nikita

2017-11-07 15:08

developer   ~0012572

pasta:

--- a/configure.ac
+++ b/configure.ac
@@ -665,9 +665,9 @@ LIBS=$SAVE_LIBS
 
 
 # libidn
-AC_MSG_CHECKING([if Libidn can be used])
-AC_ARG_WITH(libidn, AC_HELP_STRING([--with-libidn=[DIR]],
- [Support IDN (needs GNU Libidn)]),
+AC_MSG_CHECKING([if Libidn2 can be used])
+AC_ARG_WITH(libidn2, AC_HELP_STRING([--with-libidn2=[DIR]],
+ [Support IDN (needs GNU Libidn2)]),
 libidn=$withval, libidn=yes)
 if test "$libidn" != "no"; then
   if test "$libidn" != "yes"; then
@@ -676,12 +676,12 @@ if test "$libidn" != "no"; then
   fi
 fi
 libidn=no
-AC_CHECK_HEADER(idna.h,
- AC_CHECK_LIB(idn, stringprep_check_version,
- [libidn=yes LIBS="${LIBS} -lidn"], []), [])
+AC_CHECK_HEADER(idn2.h,
+ AC_CHECK_LIB(idn2, idn2_check_version,
+ [libidn=yes LIBS="${LIBS} -lidn2"], []), [])
 if test "$libidn" != "yes"; then
   AC_MSG_FAILURE([GNUnet requires libidn.
-libidn-1.13 should be sufficient, newer versions work too.])
+libidn2 should be sufficient, newer versions work too.])
 fi
 AC_MSG_RESULT($libidn)
 
--- a/src/dns/Makefile.am
+++ b/src/dns/Makefile.am
@@ -81,7 +81,7 @@ libgnunetdnsparser_la_SOURCES = \
  dnsparser.c
 libgnunetdnsparser_la_LIBADD = \
  $(top_builddir)/src/util/libgnunetutil.la $(XLIB) \
- -lidn
+ -lidn2
 libgnunetdnsparser_la_LDFLAGS = \
   $(GN_LIB_LDFLAGS) \
   -version-info 1:0:1
diff --git a/src/dns/dnsparser.c b/src/dns/dnsparser.c
index 36b4c36f1..81447fd9c 100644
--- a/src/dns/dnsparser.c
+++ b/src/dns/dnsparser.c
@@ -25,7 +25,7 @@
  * @author Christian Grothoff
  */
 #include "platform.h"
-#include <idna.h>
+#include <idn2.h>
 #if WINDOWS
 #include <idn-free.h>
 #endif

nikita

2017-11-10 12:57

developer   ~0012576

I think we could support both libidn and libidn2.

I don't know enough about the differences to judge what will happen with libidn2, so if anyone knows more please comment.

If no one comments in a reasonable long enough time, I'll commit something along these lines to master (be able to pick either of libidn or libidn2, defaulting to libidn).

Christian Grothoff

2018-06-24 13:44

manager   ~0013068

Supporting both sounds great.

nikita

2018-10-21 13:06

developer   ~0013281

Last edited: 2018-10-21 13:18

Please be aware that GNU libidn2 is the successor of GNU libidn. It comes with IDNA 2008 and TR46 implementation and also provides a compatibility layer for GNU libidn.

-- https://www.gnu.org/software/libidn/


LFS book and the operating systems I use make it pretty clear that you can have both, not colliding. List of files check out for both in a versioned way (libidn2, libidn).

I'd say we:
check for libidn2, and if it doesn't exist we check for libidn
if both exist we pick libidn2
if none exists, we fail and message that libidn or libidn2 are required with a preference for libidn2

I leave it up to you to check if our code is compatible with both, I just go by the statement that they have a compatibility layer.

Reading the 2.0.4 of libidn2, idn2.h and 1.34 of libidn, idna.h , the compatibility parts seem to check out.

I'll commit a patch today.

Christian Grothoff

2018-10-21 13:33

manager   ~0013283

Sounds good!

nikita

2018-10-21 19:50

developer   ~0013288

https://gnunet.org/git/gnunet.git/commit/?h=feature/libidn_libidn2&id=cd9355450d803fc8d67c6377a71644048515f435

If you get it to pass the apparent Makefile.am issue in src/util which I don't see,
this can be merged.
So far it fails to generate the src/util Makefile.in

Christian Grothoff

2018-10-25 12:02

manager   ~0013289

I've fixed the patch in the feature branch and merged it into master.

Issue History

Date Modified Username Field Change
2017-11-07 15:07 nikita New Issue
2017-11-07 15:08 nikita Note Added: 0012572
2017-11-10 12:57 nikita Note Added: 0012576
2017-11-10 12:58 nikita Assigned To => nikita
2017-11-10 12:58 nikita Status new => assigned
2018-06-24 13:44 Christian Grothoff Note Added: 0013068
2018-10-21 13:06 nikita Note Added: 0013281
2018-10-21 13:07 nikita Assigned To nikita => Christian Grothoff
2018-10-21 13:08 nikita Note Edited: 0013281
2018-10-21 13:18 nikita Note Edited: 0013281
2018-10-21 13:33 Christian Grothoff Note Added: 0013283
2018-10-21 13:37 Christian Grothoff Assigned To Christian Grothoff => nikita
2018-10-21 13:37 Christian Grothoff Severity minor => tweak
2018-10-21 13:37 Christian Grothoff Product Version => Git master
2018-10-21 19:50 nikita Note Added: 0013288
2018-10-21 19:50 nikita Assigned To nikita => Christian Grothoff
2018-10-25 12:02 Christian Grothoff Status assigned => resolved
2018-10-25 12:02 Christian Grothoff Resolution open => fixed
2018-10-25 12:02 Christian Grothoff Fixed in Version => 0.11.0
2018-10-25 12:02 Christian Grothoff Note Added: 0013289
2018-10-25 12:02 Christian Grothoff Target Version => 0.11.0
2019-02-28 11:17 Christian Grothoff Status resolved => closed