[SRU] weex crash/core dumps under Ubuntu 18.04 LTS.

The following worked for me:

Download the source from Sourceforge: https://sourceforge.net/projects/weex/files/weex/

Copy config.guess into the build directory, for example from /usr/share/automake-1.15/config.guess.

Change line 36 of configure.in to

AC_CHECK_LIB([ssl],[SSL_connect],

and run

autoconf configure.in > configure
automake --add-missing
autoreconf -vf
./configure
make
sudo make install

Now there is a /usr/local/bin/weex that doesn't crash.

Status changed to 'Confirmed' because the bug affects multiple users.

I did download the weex_2.8.3+b1_amd64.deb package from debian and extract the executable into /usr/local/bin/weex - that works too, including SSL - will that mean, ubuntu will get a working weex package some time automagically?

This also happens in ubuntu 20.04 lts

If build from source without libssl-dev also happens
alter building the package with libssl-dev installed the app works great

To avoid the problem, I moved to Unison (https://github.com/bcpierce00/unison), thus I cannot say if the problem has been fixed or not in more recent version of weex. Anybody has an update on this?

I can reproduce the problem in Focal and have also tested to confirm that Jammy onwards is not affected.

debdiff for Focal attached.

Thanks, Sudip.

This LGTM.

The proposed patch is the only upstream change for 2.8.3, where the issue was originally fixed. This original fix was removed in 2.8.3ubuntu1 because we did not have openssl 1.1 in ubuntu back then (zesty).

This is just removing our delta from the package to restore the intended bahavior.

Uploaded.

Hi Athos,

Could you please re-upload without .git/ in the changes?

It's present in the upload queue .diff [1] (and tarball), but not in the attached debdiff.

It seems to be an effect of the source package format/build, so if you manually 'rm -rf .git', that should work, and the result is visible running 'debdiff' locally.

Thanks!

Thanks for spotting that, Mauricio. Sorry for the mistake. I just followed my regular git ubuntu workflow here and did not consider the package format (1.0), which led to the mistake.

I hope it is fixed now.

Unsubscribing ubuntu-sponsors since the package is currently sitting in the SRU queue.

I can confirm the current fix works, but had a concern I'd like to bring to another reviewer for a second option, at least.
(Added to 'Other Info' for now).

 * This is actually (re)introducing SSL support back into the package,
   which was not available at Focal release, so this could be seen as
   a feature.

   However, considering the lack of SSL breaks the *non-SSL* FTP case
   (no guard in ftp_connect()) this could also be considered a bug-fix,
   as the package only has FTP (non-SSL) and SFTP (SSL) functionality.

   Perhaps fixing the SSL guards properly to prevent the segfault in
   non-SSL FTP mode is the minimal/actual bug-fix, but then there is
   little left for this package (and its docs/man would need updates).

$ cat ~/.weex/weexrc
[default]
[ubuntu]
HostName = ftp.ubuntu.com
LoginName = anonymous

Before:

$ weex ubuntu
Connect : ftp.ubuntu.com
Segmentation fault (core dumped)

After:

$ weex ubuntu
Connect : ftp.ubuntu.com
No password specified in configuration file for `ftp.ubuntu.com'. Asking ...
Password:

Workaround with `UseSSL = false` is not available
because the config option is guarded with USE_SSL.

$ cat ~/.weex/weexrc
[default]
[ubuntu]
HostName = ftp.ubuntu.com
LoginName = anonymous
UseSSL = false

$ weex ubuntu
/home/ubuntu/.weex/weexrc(5): UseSSL = false
Unrecognized parameter

@ src/config.c
 45 #ifdef USE_SSL
 46 { "UseSSL", CFG_BOOL, &host_use_SSL },

I think it's fine to reintroduce SSL, as it was never meant to be dropped. This will also introduce a new dependency (libssl1.1), but one that is basically impossible to be without on a system.

I don't think this fix is working, has anybody tried a focal build? The configure script is not being regenerated from configure.in and it still checks for the old function:

 debian/rules build
dh_testdir
dh_auto_configure
dh_auto_configure: warning: Compatibility levels before 9 are deprecated (level 5 in use)
    ./configure --build=x86_64-linux-gnu --prefix=/usr --includedir=\${prefix}/include --mandir=\${prefix}/share/man --infodir=\${prefix}/share/info --sysconfdir=/etc --localstatedir=/var --disable-silent-rules --libexecdir=\${prefix}/lib/weex --disable-maintainer-mode --disable-dependency-tracking
configure: WARNING: unrecognized options: --disable-silent-rules, --disable-maintainer-mode
checking for a BSD-compatible install... /usr/bin/install -c
(...)
checking for CRYPTO_new_ex_data in -lcrypto... yes
checking for SSL_library_init in -lssl... no
checking for dirent.h that defines DIR... yes
(...)

$ grep -E '(OPENSSL_init_ssl|SSL_library_init)' configure.in configure
configure.in: AC_CHECK_LIB([ssl],[OPENSSL_init_ssl],
configure: { $as_echo "$as_me:${as_lineno-$LINENO}: checking for SSL_library_init in -lssl" >&5
configure:$as_echo_n "checking for SSL_library_init in -lssl... " >&6; }
configure:if ${ac_cv_lib_ssl_SSL_library_init+:} false; then :
configure:char SSL_library_init ();
configure:return SSL_library_init ();
configure: ac_cv_lib_ssl_SSL_library_init=yes
configure: ac_cv_lib_ssl_SSL_library_init=no
configure:{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_ssl_SSL_library_init" >&5
configure:$as_echo "$ac_cv_lib_ssl_SSL_library_init" >&6; }
configure:if test "x$ac_cv_lib_ssl_SSL_library_init" = xyes; then :

I have built it again now to confirm it builds. Attached is my build log.

Hi Andreas,

Thanks for checking and reviewing this!

> I think it's fine to reintroduce SSL, as it was never meant to be dropped. This will also introduce a new dependency (libssl1.1), but one that is basically impossible to be without on a system.

Ack; nice catch; I missed that one.

> I don't think this fix is working, has anybody tried a focal build? The configure script is not being regenerated from configure.in and it still checks for the old function:

Yes, I've tested the changes in comment #14.

During that process I too noticed that _one_ of the configure checks uses the old function, but _another one_ uses the new; and the binary worked in the end, so it seemed OK.

That can be observed in the build log above as well:

$ curl -s https://launchpadlibrarian.net/708828003/focal_build.log | grep -n 'checking for .* in -lssl'
565:checking for SSL_library_init in -lssl... no
789:checking for OPENSSL_init_ssl in -lssl... yes

The first one is in dh_auto_configure, and the second one in dh_auto_build, which runs autoconf.

Cool, confirmed working.

Hello Hans, or anyone else affected,

Accepted weex into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/weex/2.8.3ubuntu2.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Looks like the package at focal-proposed is not working and still has the same problem.

Looking at the build log at https://launchpadlibrarian.net/711120570/buildlog_ubuntu-focal-amd64.weex_2.8.3ubuntu2.1_BUILDING.txt.gz I can see that it has not used the libssl and the generated control file at the end of the log shows that it only has "Depends: libc6 (>= 2.15)".

But, otoh, the build log at https://launchpadlibrarian.net/708828003/focal_build.log shows that it has used libssl and the generated control file at the end of the build has "Depends: libc6 (>= 2.15), libssl1.1 (>= 1.1.0)".

I have also built in a PPA now and the build log has the same problem.

I started looking into this, and plan to follow up on Thursday.

Thanks Mauricio. But imho, a build for -updates release can not depend on -proposed or else it can be un-installable. The problem in my local build was that mk-sbuild will have -proposed enabled by default when the schroot is created. I have now removed -prposed from my local build environments and have confirmed that I can see the same error. I will revisit this bug this weekend.

Hi Sudip, thanks for looking!
The issue doesn't seem to be related to build-time dependencies (-updates or -proposed pockets), but actually to some timestamps in the source package.
We have a new upload for focal-proposed; hopefully that should help!

What a journey! :)
The issue comes down to the timestamps of files configure.in and aclocal.m4.

The Makefile runs autoconf and runs configure again in dh_auto_build (which actually picks up libssl correctly) *if* configure.in is _more recent_ than aclocal.m4 (verified with `make --debug`), *but* the source package upload modified/updated their timestamps to _the same_.

This is unfortunate, unexpected, and likely unintended, but it *is* the build behavior in Bionic, Focal, Jammy, and Mantic/Noble (Debian sid).
(See details, logs, and tests in the next comments.)

I prepared an incremental upload for focal-proposed with the timestamps in correct order, since this is already "applied" in newer releases (essentially just restores the previous/existing behavior which changed with the last upload to focal-proposed), verified in a PPA [1].

And, of course, filed a bug in Debian [2] for awareness and suggested a patch.

cheers,
Mauricio

[1] ppa:mfo/lp1811817-proposed
[2] http://bugs.debian.org/1063565

Bionic:
---
 $ pull-lp-source weex bionic
 Found weex 2.8.3ubuntu2 in bionic
 ...
 $ ls -l --full-time weex-2.8.3ubuntu2/{aclocal.m4,configure.in}
 ... 2014-09-19 18:35:12.000000000 -0300 weex-2.8.3ubuntu2/aclocal.m4
 ... 2017-02-10 04:53:56.000000000 -0200 weex-2.8.3ubuntu2/configure.in

 $ curl -s 'https://launchpadlibrarian.net/355882845/buildlog_ubuntu-bionic-amd64.weex_2.8.3ubuntu2_BUILDING.txt.gz' | gzip -dc | grep -e '\-lssl' -e '^dh_auto[^ ]\+$'
 dh_auto_configure
 checking for SSL_library_init in -lssl... no
 dh_auto_build
 checking for SSL_library_init in -lssl... no
 dh_auto_install

Focal:
---
 $ pull-lp-source weex focal
 Found weex 2.8.3ubuntu2.1 in focal
 ...
 $ ls -l --full-time weex-2.8.3ubuntu2.1/{aclocal.m4,configure.in}
 ... 2024-01-03 12:00:07.000000000 -0300 weex-2.8.3ubuntu2.1/aclocal.m4
 ... 2024-01-03 12:00:07.000000000 -0300 weex-2.8.3ubuntu2.1/configure.in

 $ curl -sL 'https://launchpad.net/ubuntu/+source/weex/2.8.3ubuntu2.1/+build/27684312/+files/buildlog_ubuntu-focal-amd64.weex_2.8.3ubuntu2.1_BUILDING.txt.gz' | gzip -dc | grep -e '\-lssl' -e '^dh_auto[^ ]\+$'
 dh_auto_configure
 checking for SSL_library_init in -lssl... no
 dh_auto_build
 dh_auto_install

Jammy:
---
 $ pull-lp-source weex jammy
 Found weex 2.8.4.1 in jammy
 ...
 $ ls -l --full-time weex-2.8.4.1/{aclocal.m4,configure.in}
 ... 2014-09-19 18:35:12.000000000 -0300 weex-2.8.4.1/aclocal.m4
 ... 2017-02-08 12:03:36.000000000 -0200 weex-2.8.4.1/configure.in

 $ curl -sL 'https://launchpadlibrarian.net/578075516/buildlog_ubuntu-jammy-amd64.weex_2.8.4.1_BUILDING.txt.gz' | gzip -dc | grep -e '\-lssl' -e '^dh_auto[^ ]\+$'
 dh_auto_configure
 checking for SSL_library_init in -lssl... no
 dh_auto_build
 checking for OPENSSL_init_ssl in -lssl... yes
 gcc ... -o weex ... -lssl -lcrypto
 dh_auto_install

Mantic/Noble (Debian sid)
---
 $ pull-debian-source weex sid # noble
 Found weex 2.8.4.2 in sid
 ...
 $ ls -l --full-time weex-2.8.4.2/{aclocal.m4,configure.in}
 ... 2014-09-19 18:35:12.000000000 -0300 weex-2.8.4.2/aclocal.m4
 ... 2017-02-08 12:03:36.000000000 -0200 weex-2.8.4.2/configure.in

 $ curl -s 'https://buildd.debian.org/status/fetch.php?pkg=weex&arch=amd64&ver=2.8.4.2&stamp=1661206322&raw=1' | grep -e '\-lssl' -e '^dh_auto[^ ]\+$'
 dh_auto_configure
 checking for SSL_library_init in -lssl... no
 dh_auto_build
 checking for OPENSSL_init_ssl in -lssl... yes
 gcc ... -o weex ... -lssl -lcrypto
 dh_auto_install

Focal FIXUP (PPA):
---
 $ pull-ppa-source --ppa mfo/lp1811817-proposed weex focal
 Found weex 2.8.3ubuntu2.2 in focal
 ...
 $ ls -l --full-time weex-2.8.3ubuntu2.2/{aclocal.m4,configure.in}
 ... 2024-01-03 12:00:07.000000000 -0300 weex-2.8.3ubuntu2.2/aclocal.m4
 ... 2024-02-09 06:46:56.000000000 -0300 weex-2.8.3ubuntu2.2/configure.in

 $ curl -s 'https://launchpadlibrarian.net/713552062/buildlog_ubuntu-focal-amd64.weex_2.8.3ubuntu2.2_BUILDING.txt.gz' | gzip -dc | grep -e '\-lssl' -e '^dh_auto[^ ]\+$'
 dh_auto_configure
 checking for SSL_library_init in -lssl... no
 dh_auto_build
 checking for OPENSSL_init_ssl in -lssl... yes
 gcc ... -o weex ... -lssl -lcrypto
 dh_auto_install

test setup:
---

 lxc launch ubuntu:focal focal-weex
 lxc shell focal-weex

 mkdir ~/.weex
 cat <<EOF >~/.weex/weexrc
 [default]
 [ubuntu]
 HostName = ftp.ubuntu.com
 LoginName = anonymous
 EOF
 chmod 600 ~/.weex/weexrc

focal-release:
---

 apt update && apt install --yes weex

 # dpkg -s weex | grep Version:
 Version: 2.8.3ubuntu2

 # weex ubuntu
 Connect : ftp.ubuntu.com
 Segmentation fault (core dumped)

focal-proposed:
---

 add-apt-repository -y 'deb http://archive.ubuntu.com/ubuntu focal-proposed main universe' && apt install --yes weex

 # dpkg -s weex | grep Version:
 Version: 2.8.3ubuntu2.1

 # weex ubuntu
 Connect : ftp.ubuntu.com
 Segmentation fault (core dumped)

ppa:mfo/lp1811817-proposed

 add-apt-repository -y ppa:mfo/lp1811817-proposed && apt install --yes weex

 # dpkg -s weex | grep Version:
 Version: 2.8.3ubuntu2.2

 # weex ubuntu
 Connect : ftp.ubuntu.com
 No password specified in configuration file for `ftp.ubuntu.com'. Asking ...
 Password:
 ^C

$ dpkg-source -x weex_2.8.3ubuntu2.2.dsc
...

$ ls -l --full-time weex-2.8.3ubuntu2.2/{aclocal.m4,configure.in}
... 2024-01-03 12:00:07.000000000 -0300 weex-2.8.3ubuntu2.2/aclocal.m4
... 2024-02-09 06:46:56.000000000 -0300 weex-2.8.3ubuntu2.2/configure.in

$ dput ubuntu weex_2.8.3ubuntu2.2_source.changes
...
Successfully uploaded packages.

It seems like this might be a good fit for dh-autoreconf¹ to ensure that the buildsystem is actually rebuilt after patching configure.in. Otherwise this risks silently regressing on further uploads. Is there a particular reason that dh-autoreconf has not been used here?

¹: https://manpages.debian.org/testing/dh-autoreconf/dh-autoreconf.7.en.html

> This will also introduce a new dependency (libssl1.1), but one that is basically impossible to be without on a system.

To expand on this, I considered whether there could be some user who will report a regression that they have something that conflicts with libssl1.1 that we would introduce in this SRU. I verified that ubuntu-minimal transitively depends on libssl1.1, so I think that's impossible or at least unreasonable to support.

> Is there a particular reason that dh-autoreconf has not been used here?

I've now read Chris' question. This is a good point, so this bug remains blocked on this point.

Hi Chris, Robie,

>> Is there a particular reason that dh-autoreconf has not been used here?
>
> I've now read Chris' question. This is a good point, so this bug remains blocked on this point.

Sorry for the review round-trips, I should have clarified this in comment #27 (not only in Debian bug [1]).

The main reason is that weex FTBFS with dh-autoreconf (at least on Focal, and also Noble; see [1,2]).
The amount of work to address that seems to be reasonably bigger than a drive-by contribution from a SRU.

The other reasons are I considered for accepting the proposed change (and producing/uploading the fix-up):
0) upstream/Debian use the change to configure.in and rely on the timestamp-based build behavior
1) it would not be a minimal change IIUIC (despite being reasonable as explained by Chris)
2) it would require SRUs to newer releases as well (plus upload to devel) that are not strictly required
3) such SRUS would likely just remain staged with block-proposed-<release> anyway (based on weex upload history -- only Focal had SRUs among the still supported releases)

So, ignoring for a moment the FTBFS aspect, since the targeted change to configure.in (as done in Debian/upstream) and the fix-up via timestamp adjustment seemed like reasonable options, for being simpler than dh-autoreconf (although being fragile as I alluded to), and essentially bringing the package back to (fragile) consistency with other/newer releases.

cheers,
Mauricio

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063565
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=929050

We have had multiple attempts at this now. When I first reviewed this, my build with the patch didn't fix the problem, whereas other builds did. Then it was accepted, and manual verification showed that it was not fixed. I think we need something in the package to make sure this doesn't happen again. I understand the arguments against dh-autoreconf, and that might introduce other changes we won't be aware of (a diff of a rendered configure file is very hard to read). So maybe we need something else.

Some ideas that come to mind:
a) a dep8 test. The [test plan] as is could be made into a dep8 test;
b) some sort of build-time check. Something simple like making sure the final binary is linked with libssl1.1.

I like your idea to add tests, considering the difficulties with dh-autoreconf.

Between a) dep8 tests and b) build-time tests, I think b) is more helpful in the sense that it fails early enough (and could even prevent acceptance into -proposed).