Aliases validation, headers and config merging problems

[graste]

Converjon started via: converjon --config /etc/converjon/config.yaml --config-dir /etc/converjon/aliases

An alias config file like /etc/converjon/aliases/local.yaml e.g. contains:

name: local
base_file_path: "/srv/www/devbox.local/data/files"
fallback_base_file_paths:
  - "/srv/www/devbox.local/data/temp_files"
headers:
  Cache-Control: "public, max-age=86400"

Observations:

• Calling https://devbox.local/converjon/?file=local:existing.jpg works as expected and sets Cache-Control header to the given value.
• Calling https://devbox.local/converjon/?file=asdf:existing.jpg delivers the same file with the same cache headers.
• Calling https://devbox.local/converjon/?file=asdf:non-existing.jpg gives 400 with "No fallback path found"
• Adding another alias file with a different name to the aliases folder doesn't make it clear what paths or fallback paths are even being used – regardless of existing or non-existant aliases being used in the url.

The above behaviour leads me to thinking that the alias is only checked for being present and that it's not taken into account correctly. This may be a bug in the alias file handling itself OR in the combination of --config and --config-dir on startup. Maybe something else entirely.

[selfawaresoup]

The correct syntax for that alias config would be:

alias: local

base_file_path: "/srv/www/devbox.local/data/files"

fallback_base_file_paths:
  - "/srv/www/devbox.local/data/temp_files"

headers:
  Cache-Control: "public, max-age=86400"

Because there was actually no alias configured this config file is actually applied to all requests. Because of the way the config merging works, the base_file_path is now the default for all aliases, even those that haven't been configured.

One possible solution would be to not allow or ignore the base_file_path setting in a config file that has no alias in it. This however would be a breaking change.

Another way would be to check, if a requested alias actually exists, at runtime and throw an error if it doesn't. This could be seen as a bug fix because the docs don't exactly specify this behaviour.