Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

103: failed 'https://räksmörgås.se' -g '{default:puny:url}' #272

Closed
abbbi opened this issue Feb 20, 2024 · 7 comments · Fixed by #276
Closed

103: failed 'https://räksmörgås.se' -g '{default:puny:url}' #272

abbbi opened this issue Feb 20, 2024 · 7 comments · Fixed by #276

Comments

@abbbi
Copy link
Contributor

abbbi commented Feb 20, 2024

hi there,

attempting to build the latest trurl verison on debian/sid fails (linking against libcurl-gnutls (8.6.0-3)) because the
test bail:

cc -g -O2 -ffile-prefix-map=/build/trurl-0.10=. -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection $(curl-config --cflags) -W -Wall -Wshadow -Werror -pedantic -Wconversion -Wmissing-prototypes -Wwrite-strings -Wsign-compare -Wno-sign-conversion -g -Wdate-time -D_FORTIFY_SOURCE=2  -c -o trurl.o trurl.c
cc trurl.o -o trurl -Wl,-z,relro -Wl,-z,now $(curl-config --libs)
make[1]: Leaving directory '/build/trurl-0.10'
   dh_auto_test
        make -j16 test
make[1]: Entering directory '/build/trurl-0.10'
103: failed     'https://räksmörgås.se' -g '{default:puny:url}'
--- stderr --- 
expected:
''
got:
'trurl note: invalid url [Bad hostname]\n'
--- returncode --- 
expected:
0
got:
-7
--- stdout --- 
expected:
'https://xn--rksmrgs-5wao1o.se:443/\n'
got:
'\n'
104: failed     'https://räksmörgås.se' -g '{puny:url}'
--- stderr --- 
expected:
''
got:
'trurl note: invalid url [Bad hostname]\n'
--- returncode --- 
expected:
0
got:
-7
--- stdout --- 
expected:
'https://xn--rksmrgs-5wao1o.se/\n'
got:
'\n'
105: failed     'https://räksmörgås.se' -g '{puny:host}'
--- stderr --- 
expected:
''
got:
'trurl note: Bad hostname (host)\n\n'
--- returncode --- 
expected:
0
got:
0

am i missing something?

@abbbi
Copy link
Contributor Author

abbbi commented Feb 20, 2024

it appears the tests fail if LC_ALL=C is set in the build environment :-)

root@cefix:/trurl# export LC_ALL=C
root@cefix:/trurl# make test >/dev/null 2>&1; echo $?
2
root@cefix:/trurl# export -n LC_ALL
root@cefix:/trurl# make test >/dev/null 2>&1; echo $?
0

locale set in LC_* must be UTF8 (LC_ALL=C.UTF8 will work)

@jacobmealey
Copy link
Contributor

This is interesting, thanks for digging in. Just be clear, these tests past under the same paramaters in previous versions of trurl? I would need to look at what libcurl / libidn is doing, but I would imagine we may need to convert to utf8 before trying to call the lookup function. Here is the line where libcurl actually makes the call, and according the docs it's expecting a UTF-8 encoded string: https://www.gnu.org/software/libidn/libidn2/manual/html_node/Library-Functions.html#Core-Functions

Perform IDNA2008 lookup string conversion on domain name src , as described in section 5 of RFC 5891. Note that the input string must be encoded in UTF-8 and be in Unicode NFC form.

it looks like idn2 has some functions for converting from the current locale to utf8 which may just need to be called before libcurl passes the data into the lookup function above. Another hacky "solution" is just have trurl check if its using utf8 and if not, disable the punycode features.

its strange this hadn't popped up in the past, so it very well may be something silly happening on the trurl side of things.

@abbbi
Copy link
Contributor Author

abbbi commented Feb 22, 2024

have not tested or seen the issue with older releases. For building Debian packages we usually use chroots and tooling like cowbuilder or debspawn. I have changed my build environment to use cowbuilder, which seems to set defaults to LC_ALL=C, thus its the first time the issue hit me.

@jacobmealey
Copy link
Contributor

Okay that makes sense, It's probably something we broke. I'll start looking into this, thanks!

@jacobmealey
Copy link
Contributor

Okay, it looks like after some digging this has been an issue for a while (probably since punycode was introduced to trurl).

it appears the tests fail if LC_ALL=C is set in the build environment :-)

It turns out it doesn't matter what it is in the build environment, it happens at run time. the area where this issue takes place is here where it sets the locale to whatever the global locale is:

trurl/trurl.c

Line 1548 in 1074fef

setlocale(LC_ALL, "");

Earlier I had said it seems like it could be an issue with libcurl, but it looks like they are already handling locale stuff fine - I got confused because there were a lot of functions that do pretty similar things in libidn2. There are two general paths I think we can take:

  1. Modify the tests to either skip them tests that require UTF8 if the global locale doesn't support UTF8 or override the LC_ALL environment variables. I'm leaning towards the former because these tests could be ran on a machine where *.UTF8 may not exist, so just overriding the locale may result in more failures.
  2. If we are not in a UTF8 locale then don't report punycode as a feature in the output for --version.

@abbbi
Copy link
Contributor Author

abbbi commented Feb 26, 2024

It turns out it doesn't matter what it is in the build environment, it happens at run time. the area where this issue takes place is here where it sets the locale to whatever the global locale is

yes, sorry, my last post might be a bit misleading. Build + test are run in the same environment during automatic
debian package built, so yes, this is an runtime error, not an build issue.

@jacobmealey
Copy link
Contributor

@abbbi I opened #276 which i think should fix the issues you're seeing when doing the builds. Let me know if this is doesn't work for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants