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

lighttpd 1.4.71 #917

Merged
merged 2 commits into from
Aug 19, 2023
Merged

lighttpd 1.4.71 #917

merged 2 commits into from
Aug 19, 2023

Conversation

sevan
Copy link
Collaborator

@sevan sevan commented Aug 8, 2023

Drop LDAP support since OpenLDAP in Leopard & Tiger lack support for ldap_distroy() which the module calls.
Provide the missing parameters for getxattr() to fix build. Provide the definition of STAILQ_FOREACH if not found in <sys/queue.h> as with Tiger.
use-ipv6 is nolonger a configurable option.

Requires #805 so should be added to the OpenSSL branch or merged after.

Drop LDAP support since OpenLDAP in Leopard & Tiger lack support for
ldap_distroy() which the module calls.
Provide the missing parameters for getxattr() to fix build.
Provide the definition of STAILQ_FOREACH if not found in <sys/queue.h>
as with Tiger.
use-ipv6 is nolonger a configurable option.
@gstrauss
Copy link

gstrauss commented Aug 9, 2023

Hi @sevan Thanks for putting this together. I am a lighttpd developer and will accept (reasonable) portability patches upstream, such as preprocessor options like

#if defined(__APPLE__) && defined(__MACH__)
#include <TargetConditionals.h> /* TARGET_OS_IPHONE, TARGET_OS_MAC */
...
#endif

in order to use a different function signature for getxattr() where needed for older MacOS, though I do not have access to such systems on which to test, and am not up-to-speed on digging through Apple developer docs for this historical information. As an option, you can also build lighttpd without --with-attr, as I do not believe that xattr is widely used with web servers.

Is there a reason the openldap libraries on Leopard and Tiger are too old to have ldap_destroy()? Could openldap be updated? Of course I agree that the quick answer is to disable the optional LDAP feature in lighttpd since as you say, support for ldap_destroy() does not exist on Tiger. The change in lighttpd to use ldap_destroy() was added for https://redmine.lighttpd.net/issues/2849 A commenter noted that ldap_destroy() is not available before openldap 2.4 (released Oct 2007 !), but I have not verified that. If there is something appropriate to use instead of ldap_destroy() for the old openldap on Tiger, please let me know.

You may wish to submit your STAILQ_FOREACH patch upstream to https://github.com/litespeedtech/ls-hpack

FYI: openssl 1.1.1 will reach end-of-life this September: https://www.openssl.org/blog/blog/2023/03/28/1.1.1-EOL/

@sevan
Copy link
Collaborator Author

sevan commented Aug 9, 2023

Hi @gstrauss
I suspect my patch for getxattr() to add the missing parameters applies to more recent versions on macOS, since the signature has not changed between 10.4 where the function was introduced and 13.3.

This is the prototype in xattr.h for the latest macOS source release available (13.3).
On OS X 10.4 Tiger it's
ssize_t getxattr(const char *path, const char *name, void *value, size_t size, u_int32_t position, int options);
I haven't tried to build the latest version on a more recent macOS release --with-attr to confirm though.

Regarding OpenLDAP on Tiger, the OS itself was released in 2005

% /usr/libexec/slapd -VV
@(#) $OpenLDAP: slapd 2.2.19 $

Since it wasn't packaged already I refrained from packaging it but I guess we could package a new version of OpenLDAP and use that if it is buildable on Tiger.

OpenSSL 3 is packaged (#865), no idea how it'll be handled within Tigerbrew in terms of integration.

@sevan
Copy link
Collaborator Author

sevan commented Aug 9, 2023

@sevan
Copy link
Collaborator Author

sevan commented Aug 10, 2023

I have of course made the mistake of assuming that getxattr() is a feature in Darwin. Looks like the codebase assumes the Linux implementation and the parameters match that signature.

ssize_t getxattr(const char *path, const char *name, void value[.size], size_t size);

I'll raise the relevant changes upstream as you suggested.

@gstrauss
Copy link

gstrauss commented Aug 11, 2023

@sevan, why does your patch remove system "./autogen.sh" from the build steps?

FYI: lighttpd no longer provides an option to use libev, so that option should probably be removed from the config file.

For s.sub!(/^server\.event-handler\s*=\s*"linux-sysepoll"$/, 'server.event-handler = "select"'), "kqueue" should be preferred, if available on Tiger (and only if there were not major bugs with kqueue on MacOS way back in Tiger), or else "poll" should be preferred to "select". If you comment out the server.event-handler line in lighttpd.conf, lighttpd will be default use kqueue, if available, or else poll, if available, and finally select if none of the others are available.

I've added the following patch for the next release of lighttpd. Compile-tested on MacOS 11 (Big Sur):

--- a/src/stat_cache.c
+++ b/src/stat_cache.c
@@ -841,8 +841,13 @@ static int stat_cache_attr_get(const char *name) {
   #if defined(HAVE_XATTR)
    #if defined(HAVE_SYS_XATTR_H)
     ssize_t attrlen;
+   #if defined(__APPLE__) && defined(__MACH__)
+    if (0 < (attrlen = getxattr(name, attrname,
+                                attrval, sizeof(attrval)-1, 0, 0)))
+   #else
     if (0 < (attrlen = getxattr(name, attrname,
                                 attrval, sizeof(attrval)-1)))
+   #endif
    #else
     int attrlen = sizeof(attrval)-1;
     if (0 == attr_get(name, attrname, attrval, &attrlen, 0))

However, unless there is a need, you probably should compile lighttpd without --with-attr. Even with the support compiled in, lighttpd does not waste effort trying to use xattr for Content-Type unless enabled in lighttpd.conf with mimetype.use-xattr = "enabled"

@sevan
Copy link
Collaborator Author

sevan commented Aug 11, 2023

I dropped the call to autogen because the stock release source archive we build from is autotoolsed up and since we don't touch any of that infra, we don't need to re-run again. The configure stage also didn't complain about automake either so it seemed safe to drop & it saves on a lenghty build process to get those things installed before you get to build lighttpd if you don't have prebuilt packages at hand.

Just a thought regarding kqueue since I haven't tried building with it, though Tiger & Leopard have kqueue, it is not so much that it's buggy but it is very old and missing functionality found in newer versions so like the case for Dovecot, which uses ioloop & notify from kqueue but it only checks if kqueue is at all available, build passes but when it comes to run time, it fails. Are you explicitly checking for the functionality from kqueue which you require or just that kqueue is available?

I'll drop the attr & libev bits in the formula.

@gstrauss
Copy link

gstrauss commented Aug 11, 2023

It if works without the ./autogen.sh step, then that's fine. The comment removed in your patch gave me some concern:

-    # autogen must be run, otherwise prebuilt configure may complain
-    # about a version mismatch between included automake and Homebrew's
-    system "./autogen.sh"

lighttpd does not explicitly check for functionality from kqueue(), but only uses basic kqueue() functionality by default: EVFILT_READ or EVFILT_WRITE with EV_ADD or EV_DELETE. lighttpd can be configured (server.stat-cache-engine = "kqueue") to use kqueue() for EVFILT_VNODE, but that is not done by default and EVFILT_VNODE and flags are defined in the Apple developer man page for kqueue() dated Apr 14, 2000. I do not have access to ancient Mac OS Tiger systems on which to test, so can not test the kqueue() functionality, and I also can not test the sendfile() functionality, so I leave those to your discretion.

@gstrauss
Copy link

gstrauss commented Aug 11, 2023

I'd recommend removing --with-bzip2 from the build, too. In practice, bzip2 is not widely used with Accept-Encoding, if at all. brotli is much more popular.

Based on feedback in PR mistydemeo#917
Drop attr support
Building with libev is no longer a supported option
Use kqueue support as it's the optimum means
sendfile support is officially supported in 10.5 but is actually
there in 10.4 but guarded off, one of the parameters is a different
type though. Play it safe.
bzip2 encoding was non-standard so skip using it.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding
@sevan
Copy link
Collaborator Author

sevan commented Aug 11, 2023

lighttpd does not explicitly check for functionality from kqueue(), but only uses basic kqueue() functionality by default: EVFILT_READ or EVFILT_WRITE with EV_ADD or EV_DELETE. lighttpd can be configured (server.stat-cache-engine = "kqueue") to use kqueue() for EVFILT_VNODE, but that is not done by default and EVFILT_VNODE and flags are defined in the Apple developer man page for kqueue() dated Apr 14, 2000. I do not have access to ancient Mac OS Tiger systems on which to test, so can not test the kqueue() functionality, and I also can not test the sendfile() functionality, so I leave those to your discretion.

All those are available in sys/event.h so I enabled it. Skipping on sendfile as it is technically there but guarded off (officially it was introduced in 10.5).

@gstrauss
Copy link

@sevan your changes look good to me.

With the removal of --with-attr, you probably should remove your patch to stat_cache.c

-                                attrval, sizeof(attrval)-1)))
+                                attrval, sizeof(attrval)-1, 0, NULL)))

or else that will conflict with future releases of lighttpd (which will contain the patch I posted further above)

FYI: if the input file is lighttpd source doc/config/lighttpd.conf, then you do not have to uncomment server.event-handler, as that will use the best OS event framework available, i.e. kqueue on Mac OS.

FYI: If you're not sure about sendfile() support, it is still a good idea to specify server.network-backend = "writev" since if not specified, and presence of sendfile() is detected, then sendfile() will be used by default by lighttpd to send files. However, if sendfile() fails, lighttpd will transparently fallback to writev(), so if sendfile() always fails, it will waste a syscall each time sending files to rediscover sendfile() will fail.

@sevan
Copy link
Collaborator Author

sevan commented Aug 11, 2023

Decided to leave the patch there should someone want to build with attr support. It'll be dropped when the formula is upgraded in the future as the patch won't apply.

@mistydemeo
Copy link
Owner

Thank you so much everyone for all the work on this. Just to check, is this PR now ready for merge/binary builds?

@gstrauss
Copy link

LGTM. Thanks!

@sevan
Copy link
Collaborator Author

sevan commented Aug 12, 2023

Yep, ready to land.

lighttpd-git pushed a commit to lighttpd/lighttpd1.4 that referenced this pull request Aug 13, 2023
Copy link
Owner

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@mistydemeo mistydemeo merged commit 42bcaf2 into mistydemeo:master Aug 19, 2023
@sevan sevan deleted the lighttpd1471 branch August 31, 2023 13:09
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 this pull request may close these issues.

3 participants