Project

General

Profile

Feature #411

typo in config file did not show up in a config-test

Added by Anonymous over 14 years ago. Updated over 4 years ago.

Status:
Fixed
Priority:
Normal
Category:
core
Target version:
ASK QUESTIONS IN Forums:

Description

simple-vhost.default-root = "grisu.home.kneschke.de"

did not generate a warning during the config test (note, root should read host)

Howver, when I started the daemon, I got a warning about an invalid config key (or some such)

simple-vhost.default-root = "grisu.home.kneschke.de"
nero# lighttpd -t -f lighttpd.conf
Syntax OK

nero# ./rc.d/lighttpd.sh restart
Stopping lighttpd.
Starting lighttpd.
nero# 2005-12-09 16:24:57: (server.c.693) WARNING: unknown config-key: simple-vhost.default-root (ignored)

Perhaps keys are not checked during startup?

lighttpd-1.4.8 (ssl) - a light and fast webserver
Build-Date: Dec 9 2005 10:54:27

-- eydaimon

#1

Updated by Anonymous over 14 years ago

adding myself to the CC

-- eydaimon

#2

Updated by stbuehler almost 12 years ago

  • Status changed from New to Fixed
  • Resolution set to invalid

As some modules do more than just checking the syntax while loading the config, we won't change that. And the syntax of your file is ok...

#3

Updated by Anonymous almost 12 years ago

  • Status changed from Fixed to Need Feedback
  • Resolution deleted (invalid)

Then I suggest that:

-t         test the config-file, and exit

is changed to read:

-t         test config-file for syntax errors, and exit
#4

Updated by gstrauss over 4 years ago

  • Description updated (diff)

stbuehler, what would you think of extending the -t flag so that if provided more than once on command line (e.g. "-tt" or "-t -t"), that config processing continues, loads all modules, and exits after checking for unused/deprecated config keys?

Anything that with potentially destructive behavior would have to be skipped, such as overwriting pidfile. daemonizing would be disabled to keep process in foreground and stderr attached. network_init() would need to be passed an extra flags to indicate "check_only" so as not to attempt to bind() to potentially in-use addresses. plugin hooks 'init' and 'set_defaults', called by plugins_call_init() and plugins_call_set_defaults(), would need to take an "check-only" flag, too, so this would be a breaking change to third-party modules, similar to the change in 1.4.38 in r3049 which extended function signature of config_insert_values_internal(). For most modules, they would still perform their complete setup routine, but there might be a few which need to skip a few actions and the "check-only" flag would communicate that.

#5

Updated by gstrauss over 4 years ago

Instead of changing the function signatures, a much less invasive way to make the change would be to add a flag 'preflight_check' to server_config (srv->srvconf) (e.g. right along with 'dont_daemonize' member)

I looked through the code and these places would need to check for srv->srvconf.preflight_check:
server.c:main()
skip pidfile creation
skip daemonizing
skip log_error_open()
network.c:network_server_init()
skip bind() to sockets

Skipping error log and access log opens is not technically required for a startup test, but probably still a good idea to avoid triggering custom behavior by piped loggers
server.c:main()
skip log_error_open()
mod_accesslog.c:log_access_open()
skip open_log_file_or_pipe()

stbuehler, think you would accept a patch for this if I created one?

#6

Updated by stbuehler over 4 years ago

  • Tracker changed from Bug to Feature
  • Assignee deleted (jan)

gstrauss: No promises on acceptance or review time :) But I'm open to the idea.

#7

Updated by gstrauss over 4 years ago

  • Status changed from Need Feedback to Patch Pending
#8

Updated by gstrauss over 4 years ago

Turned out not to be a very large change:
https://github.com/lighttpd/lighttpd1.4/pull/44

 src/base.h          |  1 +
 src/mod_accesslog.c |  2 ++
 src/network.c       | 12 ++++++++++--
 src/server.c        | 23 ++++++++++++++++++++---
 4 files changed, 33 insertions(+), 5 deletions(-)

#9

Updated by stbuehler over 4 years ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100

Applied in changeset r3130.

#10

Updated by stbuehler over 4 years ago

  • Target version set to 1.4.40

Also available in: Atom