Skip to content

Commit

Permalink
Fix stack overflow triggered by casefolding script (#405)
Browse files Browse the repository at this point in the history
Seperates setting up logging from parsing the config file
  • Loading branch information
Azrenbeth authored Sep 28, 2021
1 parent 7a60521 commit bf83fe1
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 59 deletions.
1 change: 1 addition & 0 deletions changelog.d/405.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Move the configuration file handling code into a separate module.
45 changes: 1 addition & 44 deletions sydent/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
from configparser import DEFAULTSECT, ConfigParser
from typing import Dict

from twisted.python import log

from sydent.config.crypto import CryptoConfig
from sydent.config.database import DatabaseConfig
from sydent.config.email import EmailConfig
Expand Down Expand Up @@ -215,7 +213,7 @@ def parse_from_config_parser(self, cfg: ConfigParser) -> bool:
def parse_config_file(self, config_file: str) -> None:
"""
Parse the given config from a filepath, populating missing items and
sections. NOTE: this method also sets up logging.
sections.
:param config_file: the file to be parsed
"""
Expand All @@ -236,13 +234,8 @@ def parse_config_file(self, config_file: str) -> None:

cfg.read(config_file)

# Logging is configured in cfg, but these options must be parsed first
# so that we can log while parsing the rest
setup_logging(cfg)

# TODO: Don't alter config file when starting Sydent so that
# it can be set to read-only

needs_saving = self.parse_from_config_parser(cfg)

if needs_saving:
Expand Down Expand Up @@ -271,40 +264,4 @@ def parse_config_dict(self, config_dict: Dict) -> None:
for option, value in section_dict.items():
cfg.set(section, option, value)

# This is only ever called by tests so don't configure logging
# as tests do this themselves

self.parse_from_config_parser(cfg)


def setup_logging(cfg: ConfigParser) -> None:
"""
Setup logging using the options selected in the config
:param cfg: the configuration
"""
log_format = "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s" " - %(message)s"
formatter = logging.Formatter(log_format)

logPath = cfg.get("general", "log.path")
if logPath != "":
handler: logging.StreamHandler = logging.handlers.TimedRotatingFileHandler(
logPath, when="midnight", backupCount=365
)
handler.setFormatter(formatter)

def sighup(signum, stack):
logger.info("Closing log file due to SIGHUP")
handler.doRollover()
logger.info("Opened new log file due to SIGHUP")

else:
handler = logging.StreamHandler()

handler.setFormatter(formatter)
rootLogger = logging.getLogger("")
rootLogger.setLevel(cfg.get("general", "log.level"))
rootLogger.addHandler(handler)

observer = log.PythonLoggingObserver()
observer.start()
9 changes: 3 additions & 6 deletions sydent/config/crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import logging
from configparser import ConfigParser

import nacl
import signedjson.key

from sydent.config._base import BaseConfig

logger = logging.getLogger(__name__)


class CryptoConfig(BaseConfig):
def parse_config(self, cfg: "ConfigParser") -> bool:
Expand All @@ -36,8 +33,8 @@ def parse_config(self, cfg: "ConfigParser") -> bool:
save_key = False

if signing_key_str == "":
logger.info(
"This server does not yet have an ed25519 signing key. "
print(
"INFO: This server does not yet have an ed25519 signing key. "
"Creating one and saving it in the config file."
)

Expand All @@ -46,7 +43,7 @@ def parse_config(self, cfg: "ConfigParser") -> bool:
save_key = True
elif len(signing_key_parts) == 1:
# old format key
logger.info("Updating signing key format: brace yourselves")
print("INFO: Updating signing key format: brace yourselves")

self.signing_key = nacl.signing.SigningKey(
signing_key_str, encoder=nacl.encoding.HexEncoder
Expand Down
19 changes: 10 additions & 9 deletions sydent/config/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import logging
import os
from configparser import ConfigParser
from typing import List
Expand All @@ -23,8 +22,6 @@
from sydent.config._base import BaseConfig
from sydent.util.ip_range import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set

logger = logging.getLogger(__name__)


class GeneralConfig(BaseConfig):
def parse_config(self, cfg: "ConfigParser") -> bool:
Expand All @@ -36,12 +33,15 @@ def parse_config(self, cfg: "ConfigParser") -> bool:
self.server_name = cfg.get("general", "server.name")
if self.server_name == "":
self.server_name = os.uname()[1]
logger.warning(
"You have not specified a server name. I have guessed that this server is called '%s'. "
"If this is incorrect, you should edit 'general.server.name' in the config file."
% (self.server_name,)
print(
"WARNING: You have not specified a server name. I have guessed that this "
f"server is called '{self.server_name}'. If this is incorrect, you should "
"edit 'general.server.name' in the config file."
)

self.log_level = cfg.get("general", "log.level")
self.log_path = cfg.get("general", "log.path")

# Get the possible brands by looking at directories under the
# templates.path directory.
self.templates_path = cfg.get("general", "templates.path")
Expand All @@ -52,8 +52,9 @@ def parse_config(self, cfg: "ConfigParser") -> bool:
if os.path.isdir(os.path.join(self.templates_path, p))
}
else:
logging.warning(
f"The path specified by 'general.templates.path' ({self.templates_path}) does not exist."
print(
f"WARNING: The path specified by 'general.templates.path' ({self.templates_path}) "
"does not exist."
)
# This is a legacy code-path and assumes that verify_response_template,
# email.template, and email.invite_template are defined.
Expand Down
32 changes: 32 additions & 0 deletions sydent/sydent.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import twisted.internet.reactor
from twisted.internet import address, task
from twisted.python import log

from sydent.config import SydentConfig
from sydent.db.hashing_metadata import HashingMetadataStore
Expand Down Expand Up @@ -306,9 +307,40 @@ def run_gc():
gc.collect(i)


def setup_logging(config: SydentConfig) -> None:
"""
Setup logging using the options specified in the config
:param config: the configuration to use
"""
log_path = config.general.log_path
log_level = config.general.log_level

log_format = "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s" " - %(message)s"
formatter = logging.Formatter(log_format)

if log_path != "":
handler: logging.StreamHandler = logging.handlers.TimedRotatingFileHandler(
log_path, when="midnight", backupCount=365
)
handler.setFormatter(formatter)

else:
handler = logging.StreamHandler()

handler.setFormatter(formatter)
rootLogger = logging.getLogger("")
rootLogger.setLevel(log_level)
rootLogger.addHandler(handler)

observer = log.PythonLoggingObserver()
observer.start()


if __name__ == "__main__":
sydent_config = SydentConfig()
sydent_config.parse_config_file(get_config_file_path())
setup_logging(sydent_config)

syd = Sydent(sydent_config)
syd.run()

0 comments on commit bf83fe1

Please sign in to comment.