(Sorry Omar, I accidentally replied directly off-list)

On 23/01/06 12:53, Omar Polo wrote:
> Hello,
>
> On 2023/01/04 21:13:29 -0700, Ashlen <euryd...@riseup.net> wrote:
> > Updated a couple of things.
> >
> > - Use '&&' instead of ';' so that sed only executes after a successful cd.
> >
> > - Needed to patch version string in src/luasocket.h, I guess I left that 
> > out for
> >   some reason.
> >
> > - Add TEST_DEPENDS so that make test only works if net/luasocket is 
> > installed.
> >
> > Is this OK? Any feedback?
>
> thanks for looking at luasocket.

Of course. I was looking into a separate issue and saw it was out of date. I run
net/prosody and want the dependencies to be up to date.

(Off-topic: that separate issue that caused me to end up here is an interaction
between security/luasec and net/prosody that results in Secure Client-Initiated
Renegotiation being enabled by default instead of disabled. I found it with
testssl.sh. Allegedly that's a potential DoS risk, so I'd also like to fix that
at some point, but as I said it's unrelated. Would be content to go into more
detail about that in a separate mail on ports@).

> The diff looks mostly OK to me, a few considerations:
>
>  - Fails to package with lua != 5.1; need to set LUAV=5.X for it to
>    work (try `FLAVOR=lua52 make package')

Thanks for pointing that out, I didn't think about that. Reproduced that failure
locally.

>  - Maybe we can drop the buffer_* -> ls_buffer_* rename?  FreeBSD had
>    similar patches and they dropped them with the update to 3.0.0[0].
>    Unfortunately the commit message doesn't explain why.
>
>    A quick compare of `nm /usr/local/lib/lighttpd/mod_magnet.so' with
>    the exported symbols from buffer.c show no overlap, but I haven't
>    run-tested.

I think "buffer_init" is the thing that was said to cause a problem, although I
haven't run-tested either. mod_magnet at the time of that commit (way back in
2009) appears to call it, but you're right, it doesn't anymore. Though, other
stuff in lighttpd might, so I wanted to check and see if it needs more
investigating.

I installed lighttpd and checked out version 3.1.0 of luasocket. Here I'm
grabbing all of the symbols that start with buffer_.

$ nm /usr/local/lib/lighttpd/mod_*.so \
        | perl -nE 'chomp; next unless /\bbuffer_/; @segments = split " "; say 
$segments[1]' \
        | sort -u
buffer_append_base64_decode
buffer_append_base64_enc
buffer_append_bs_escaped
[a lot more entries like this...]

I did some text filtering on this output to build a command to compare the
symbols against luasocket. Here's an echo to show what would get executed, so
the results of the eval afterward will make sense.

$ cd /path/to/luasocket_src
$ echo grep -nR "$(\
        nm /usr/local/lib/lighttpd/mod_*.so \
        | perl -nE 'chomp; next unless /\bbuffer_/; @segments = split " "; say 
$segments[1]' \
        | sort -u \
        | sed -e 's/^/-e /' -e 's/buffer_.*/\"&\"/' \
        | tr '\n' ' ' \
        )".

grep -nR -e "buffer_append_base64_decode" -e "buffer_append_base64_enc" 
[snip...] .

Replacing that echo with an eval, I get this.

./src/buffer.c:40:void buffer_init(p_buffer buf, p_io io, p_timeout tm) {
./src/buffer.h:41:void buffer_init(p_buffer buf, p_io io, p_timeout tm);
./src/tcp.c:238:        buffer_init(&clnt->buf, &clnt->io, &clnt->tm);
./src/tcp.c:409:    buffer_init(&tcp->buf, &tcp->io, &tcp->tm);
./src/tcp.c:448:    buffer_init(&tcp->buf, &tcp->io, &tcp->tm);
./src/serial.c:169:    buffer_init(&un->buf, &un->io, &un->tm);
./src/unixdgram.c:398:        buffer_init(&un->buf, &un->io, &un->tm);
./src/unixstream.c:172:        buffer_init(&clnt->buf, &clnt->io, &clnt->tm);
./src/unixstream.c:348:        buffer_init(&un->buf, &un->io, &un->tm);

So it appears "buffer_init" is the one that shows up in both. I was curious what
modules in lighttpd have buffer_init in their symbols table. Here they are:

$ for lighttpd_module in /usr/local/lib/lighttpd/mod_*.so; do
        if nm "${lighttpd_module}" | grep -qE '[[:<:]]buffer_init[[:>:]]'; then
                echo "${lighttpd_module}"
        fi
done
/usr/local/lib/lighttpd/mod_cgi.so
/usr/local/lib/lighttpd/mod_openssl.so

$ nm /usr/local/lib/lighttpd/mod_{cgi,openssl}.so | grep -E 
'[[:<:]]buffer_init[[:>:]]'
         U buffer_init
         U buffer_init

(Off-topic: if any of the commands I used or my approach to find this are flawed
in some way, feel free to point it out. Always happy to learn. I only know Perl
and shell, so my knowledge of lower level things is lacking.)

I'm unsure how to test lighttpd for that SIGSEGV at the moment. I'd need to look
(or if someone else familiar with lighttpd would be willing to test it, that'd
be great. I've never used it).

>    Anyway, patches are better than sed.  You get to see how the
>    sources are being changed and makes easier to spot unwanted
>    replacements.

Good point, I was unsure about that and actually tried both ways before
submitting the sed version. Maybe I should wait to add patches back in until
after this gets some run-testing? It seems good to confirm that it's an issue
still.

>  - `make test' can be simplified by symlinking ${MODLUA_BIN} in
>    ${WRKDIR}/bin/lua (which is in the $PATH for ports build.)  The
>    test is dummy however, it just loads the library and prints to
>    standard output.  The client and server tests are much more
>    interesting, but needs to run the server first and then the client;
>    difficult to do from a Makefile without some help.
>
>    It would be nice to upstream a patch to change that `lua hello.lua'
>    to `${LUA} hello.lua' so that we can override the executable.  it
>    would be even better if upstream provided a script to test the
>    server/client interaction.

I didn't think to do a symlink there as a workaround. Cool. :) An upstreamed
patch would be nice, yes.

>  - I wouldn't patch just to fix the version number, but won't object
>    it either since it's already done :)
>
>    Maybe you want to send them upstream too?

I can send a pull request, sure. If they accept, I can send an updated diff here
later to remove the patches.

> I've also taken the liberty to tweak a bit the Makefile, dropping
> things no more needed and making it a bit easier to read (IMHO).

Thank you, I really appreciate that.

> This needs some further testing.  Packages that depends on luasockets
> are
>
>  - devel/luacopas                no test suite
>  - devel/luaevent                test run OK
>  - net/jitsi/prosody-plugins     no test suite
>  - net/prosody                   no test suite
>  - security/luasec               no test suite
>  - www/luakit                    with simple runtesting seems fine
>
> I can manage to test it with net/prosody on -RELEASE but will take a
> while.

I'll see if I can test against net/prosody on -RELEASE as well, actually. I'll
do my best to follow your suggestions. Thank you for taking the time to respond.

Reply via email to