test1560: skip some URLs if UTF-8 is not supported

[sergiodj]

Debian CI found that lib1560 implements tests that will fail when
UTF-8 isn't supported. We can detect that with nl_langinfo and skip
the specific URLs that fail (i.e., those whose getflags are either
CURLU_PUNYCODE or CURLU_PUNY2IDN).

Co-authored-by: Viktor Szakats

[sergiodj]

Cc @samueloph @charles2910

[sergiodj]

@guilherme-puida

[vszakats]

Where is HAVE_LANGINFO_CODESET defined? I wonder if this'd need some logic for CMake for example.

[sergiodj]

Where is HAVE_LANGINFO_CODESET defined? I wonder if this'd need some logic for CMake for example.

Sorry, you're correct, I made a mistake here. This macro needs to be defined; I was first considering using USE_LIBIDN2 but I don't think that's a good idea because using libidn2 isn't necessarily an indication that UTF-8 will be used.

Let me do some work to define the HAVE_LANGINFO_CODESET macro.

[vszakats]

If there is a way to detect this via existing macros and perhaps envs, it may simplify things.

[bagder]

We probably need checks for nl_langinfo and langinfo.h added, and then add an include of the header if present.

[sergiodj]

OK, I force-pushed new changes which implement autotools and cmake checks for langinfo.h, nl_langinfo and CODESET. I'm not a cmake expert so I'd appreciate a review.

[vszakats]

It's many build-time checks, feeling a little excessive to run in every build, just to use it in a single test.

Is there a way to avoid some or all of them? Is there perhaps a runtime check that doesn't involve a call into a nl_langinfo()?

Worst case I'm thinking to reduce it to a single build-time check for nl_langinfo(CODESET), and pre-fill that (or avoid the check) for platforms that don't need or have them (or always have them) (Windows, macOS) for CMake.

[testclutch]

Analysis of PR #17933 at 91d37396:

Test 870 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 6 different CI jobs (the link just goes to one of them).

Generated by Testclutch

[vszakats]

Would it be possible to detect the lack of UTF-8 support via
runtests.pl in Perl?

Then pass the information to lib1560 via an env, or the test
command-line?

according to the man page, locale charmap returns this
information.
https://linux.die.net/man/3/nl_langinfo

[vszakats]

Or, would it work to set LC_ALL=en_US.UTF-8 for 1560, as
is done already for other tests, for example 1448?

[bagder]

Or, would it work to set LC_ALL=en_US.UTF-8 for 1560, as is done already for other tests, for example 1448?

Test 1560 already set such a thing. I believe this attempt, if I understand things correctly, is for systems without a functional UTF-8 locale so setting one doesn't make it work.

[vszakats]

Or, would it work to set LC_ALL=en_US.UTF-8 for 1560, as is done already for other tests, for example 1448?

Test 1560 already set such a thing. I believe this attempt, if I understand things correctly, is for systems without a functional UTF-8 locale so setting one doesn't make it work.

Oh, right, I was grepping for LC_ALL, while test1560 sets LANG. It's the only test doing that.

While I can't test on a system not having UTF-8 support at all. If the default isn't UTF-8,
LC_ALL is able to change it to UTF-8, while LANG isn't. As seen via locale charmap (= the command-line equivalent of nl_langinfo(CODESET):

$ export LC_ALL=C
$ locale charmap
ANSI_X3.4-1968
$ LC_ALL=en_US.UTF-8 locale charmap
UTF-8
$ LANG=en_US.UTF-8 locale charmap
ANSI_X3.4-1968

(For systems with no UTF-8 at all, I wonder how the other tests pass, that require LC_ALL=*UTF8.)

[vszakats]

I could replicate 1560 failing by setting LC_ALL=C before running the tests, on macOS. (on Linux I couldn't)

This patch fixes it:

--- a/tests/data/test1560
+++ b/tests/data/test1560
@@ -13,7 +13,7 @@ urlapi
 none
 </server>
 <setenv>
-LANG=en_US.UTF-8
+LC_ALL=en_US.UTF-8
 </setenv>
 <features>
 file

edit: Also replicates on Linux. (libidn2 wasn't enabled in my initial try)

[bagder]

This patch fixes it:

I just suspect it is not the same case as @sergiodj is trying to address here...

[vszakats]

runtests provides the codeset-utf8 feature to detect if the env supports UTF-8.

This information could be propagated via an env and used as a filter:

--- a/tests/libtest/lib1560.c
+++ b/tests/libtest/lib1560.c
@@ -1428,7 +1428,7 @@ static int set_url(void)
    2. Set one or more parts
    3. Extract and compare all parts - not the URL
 */
-static int setget_parts(void)
+static int setget_parts(int has_utf8)
 {
   int i;
   int error = 0;
@@ -1440,6 +1440,10 @@ static int setget_parts(void)
       error++;
       break;
     }
+    if((setget_parts_list[i].getflags == CURLU_PUNYCODE ||
+        setget_parts_list[i].getflags == CURLU_PUNY2IDN) && !has_utf8) {
+      continue;
+    }
     if(setget_parts_list[i].in)
       rc = curl_url_set(urlp, CURLUPART_URL, setget_parts_list[i].in,
                         setget_parts_list[i].urlflags);
@@ -1525,7 +1529,7 @@ static int set_parts(void)
   return error;
 }
 
-static int get_url(void)
+static int get_url(int has_utf8)
 {
   int i;
   int error = 0;
@@ -1536,6 +1540,10 @@ static int get_url(void)
       error++;
       break;
     }
+    if((get_url_list[i].getflags == CURLU_PUNYCODE ||
+        get_url_list[i].getflags == CURLU_PUNY2IDN) && !has_utf8) {
+      continue;
+    }
     rc = curl_url_set(urlp, CURLUPART_URL, get_url_list[i].in,
                       get_url_list[i].urlflags);
     if(!rc) {
@@ -1565,7 +1573,7 @@ static int get_url(void)
   return error;
 }
 
-static int get_parts(void)
+static int get_parts(int has_utf8)
 {
   int i;
   int error = 0;
@@ -1576,6 +1584,10 @@ static int get_parts(void)
       error++;
       break;
     }
+    if((get_parts_list[i].getflags == CURLU_PUNYCODE ||
+        get_parts_list[i].getflags == CURLU_PUNY2IDN) && !has_utf8) {
+      continue;
+    }
     rc = curl_url_set(urlp, CURLUPART_URL,
                       get_parts_list[i].in,
                       get_parts_list[i].urlflags);
@@ -2018,15 +2030,17 @@ err:
 
 static CURLcode test_lib1560(char *URL)
 {
+  int has_utf8 = !!getenv("CURL_TEST_HAVE_CODESET_UTF8");
+
   (void)URL; /* not used */
 
   if(urldup())
     return (CURLcode)11;
 
-  if(setget_parts())
+  if(setget_parts(has_utf8))
     return (CURLcode)10;
 
-  if(get_url())
+  if(get_url(has_utf8))
     return (CURLcode)3;
 
   if(huge())
@@ -2047,7 +2061,7 @@ static CURLcode test_lib1560(char *URL)
   if(set_parts())
     return (CURLcode)2;
 
-  if(get_parts())
+  if(get_parts(has_utf8))
     return (CURLcode)4;
 
   if(clear_url())
--- a/tests/runtests.pl
+++ b/tests/runtests.pl
@@ -809,6 +809,9 @@ sub checksystemfeatures {
     $feature{"crypto"} = $feature{"NTLM"} || $feature{"Kerberos"} || $feature{"SPNEGO"};
     $feature{"local-http"} = servers::localhttp();
     $feature{"codeset-utf8"} = lc(langinfo(CODESET())) eq "utf-8";
+    if($feature{"codeset-utf8"}) {
+        $ENV{'CURL_TEST_HAVE_CODESET_UTF8'} = 1;
+    }
 
     # make each protocol an enabled "feature"
     for my $p (@protocols) {

[sergiodj]

It's many build-time checks, feeling a little excessive to run in every build, just to use it in a single test.

Actually, nl_langinfo is also called (unchecked) in iconv_to_utf8, so we have precedence and could use these checks elsewhere too.

Is there a way to avoid some or all of them? Is there perhaps a runtime check that doesn't involve a call into a nl_langinfo()?

What we could do is simplify the checks. For example, assume that if nl_langinfo is available then CODESET will be too.

[vszakats]

It's many build-time checks, feeling a little excessive to run in every build, just to use it in a single test.

Actually, nl_langinfo is also called (unchecked) in iconv_to_utf8, so we have precedence and could use these checks elsewhere too.

It's for Apple platforms only.

Is there a way to avoid some or all of them? Is there perhaps a runtime check that doesn't involve a call into a nl_langinfo()?

What we could do is simplify the checks. For example, assume that if nl_langinfo is available then CODESET will be too.

Moving platform-dependent feature detection to C comes with a great
overhead (for every build session of every platform), and avoiding it would
be best, if there is a way.

The simplest would be using the detection logic already present in runtests.pl:

curl/tests/runtests.pl

Line 811 in cfbfb65

$feature{"codeset-utf8"} = lc(langinfo(CODESET())) eq "utf-8";

If this doesn't fit this case, another could be added in a similar vain
for this particular case? Then communicate it via an env to the test.
Tests are always run through runtests.pl, making it safe.

[sergiodj]

Sure, I'm OK with the patch proposed in #17933 (comment) , btw.

[vszakats]

Sure, I'm OK with the patch proposed in #17933 (comment) , btw.

If you say it works, nice! Would you prefer applying it, or shall I do?

[sergiodj]

Sure, I'm OK with the patch proposed in #17933 (comment) , btw.

If you say it works, nice! Would you prefer applying it, or shall I do?

I can do it.

[sergiodj]

FTR, I force-pushed the new changes a few days ago.

[vszakats]

Thank you @sergiodj! Merged now.