Skip to content

Commit

Permalink
bridge: Fix crash on invalid cockpit.conf
Browse files Browse the repository at this point in the history
If there is a syntax error in cockpit.conf, the bridge previously
crashed with e.g. a `configparser.MissingSectionHeaderError`.

Intercept that, log a warning instead, and ignore the config file
instead.

https://bugzilla.redhat.com/show_bug.cgi?id=2324979
  • Loading branch information
martinpitt committed Nov 15, 2024
1 parent 24225d8 commit f86615a
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
14 changes: 14 additions & 0 deletions pkg/base1/test-dbus.js
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,20 @@ Empty=
assert.rejects(proxy.GetString("SomeSection", "UnknownKey"),
/key.*UnknownKey.*not exist/,
"unknown key raises an error");

// empty config
await cockpit.file(configDir + "/cockpit/cockpit.conf").replace("");
await proxy.Reload();
assert.rejects(proxy.GetString("SomeSection", "SomeA"),
/key.*SomeSection.*not exist/,
"query in empty config raises an error");

// broken config (missing section header)
await cockpit.file(configDir + "/cockpit/cockpit.conf").replace("SomeA = two");
await proxy.Reload();
assert.rejects(proxy.GetString("SomeSection", "SomeA"),
/key.*SomeSection.*not exist/,
"query in broken config raises an error");
});

QUnit.test("nonexisting address", async assert => {
Expand Down
7 changes: 6 additions & 1 deletion src/cockpit/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@ def reload(self):
cockpit_conf = lookup_config('cockpit.conf')
logger.debug("cockpit.Config: loading %s", cockpit_conf)
# this may not exist, but it's ok to not have a config file and thus leave self.config empty
self.config.read(cockpit_conf)
try:
self.config.read(cockpit_conf)
except configparser.Error as exc:
logger.warning("cockpit.conf is invalid: %s", exc)
self.config.clear()
return


class Environment(bus.Object, interface='cockpit.Environment'):
Expand Down
15 changes: 15 additions & 0 deletions test/verify/check-connection
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,21 @@ UnixPath=/run/cockpit/session
self.login_and_go(None)
b.wait_visible("#nav-system li:contains(Hackices)")
self.assertFalse(b.is_present("#nav-system li:contains(Services)"))
b.logout()

# still starts with a broken config (no section headers)
m.write("/etc/test-xdg/cockpit/cockpit.conf", "LoginTitle=NoNoNO\n")
m.execute("systemctl stop cockpit")
self.login_and_go(None)
b.wait_visible("#nav-system li:contains(Hackices)")
self.assertFalse(b.is_present("#nav-system li:contains(Services)"))
b.logout()
# message from cockpit-ws
self.allow_journal_messages(".*cockpit.conf: key=val line not in any section.*")
# message from bridge
self.allow_journal_messages(".*cockpit.conf is invalid: File contains no section headers.*")
self.allow_journal_messages(".*cockpit.conf.*line: 1")
self.allow_journal_messages(".*LoginTitle=NoNoNO.*")

@testlib.nondestructive
@testlib.skipBeiboot("no cockpit-bridge binary on host in beiboot mode")
Expand Down

0 comments on commit f86615a

Please sign in to comment.