From 748ef2332635210983e46aab3d3ae7e6e5b487f5 Mon Sep 17 00:00:00 2001 From: syeopite Date: Tue, 3 Jun 2025 07:50:21 -0700 Subject: [PATCH] Replace `Kemal::StaticFileHandler` on Crystal < 1.17.0 Kemal's subclass of the stdlib `HTTP::StaticFileHandler` is not as maintained as its parent, and so misses out on many enhancements and bug fixes from upstream, which unfortunately also includes the patches for security vulnerabilities... Though this isn't necessarily Kemal's fault since the bulk of the stdlib handler's logic was done in a single big method, making any changes hard to maintain. This was fixed in Crystal 1.17.0 where the handler was refactored into many private methods, making it easier for an inheriting type to implement custom behaviors while still leveraging much of the pre-existing code. Since we don't actually use any of the Kemal specific features added by `Kemal::StaticFileHandler`, there really isn't a reason to not just create a new handler based upon the stdlib implementation instead which will address the problems mentioned above. This PR implements a new handler which inherits from the stdlib variant and overrides the helper methods added in Crystal 1.17.0 to add the caching behavior with minimal code changes. Since this new handler depends on the code in Crystal 1.17.0, it will only be applied on versions greater than or equal to 1.17.0. On older versions we'll fallback to the current monkey patched `Kemal::StaticFileHandler` --- src/ext/kemal_static_file_handler.cr | 21 +++ src/invidious.cr | 18 ++- .../http_server/static_assets_handler.cr | 138 ++++++++++++++++++ 3 files changed, 171 insertions(+), 6 deletions(-) create mode 100644 src/invidious/http_server/static_assets_handler.cr diff --git a/src/ext/kemal_static_file_handler.cr b/src/ext/kemal_static_file_handler.cr index a5f42261..c6b9a27d 100644 --- a/src/ext/kemal_static_file_handler.cr +++ b/src/ext/kemal_static_file_handler.cr @@ -1,3 +1,24 @@ +{% if compare_versions(Crystal::VERSION, "1.17.0") >= 0 %} + # Strip StaticFileHandler from the binary + # + # This allows us to compile on 1.17.0 as the compiler won't try to + # semantically check the outdated upstream code. + class Kemal::Config + private def setup_static_file_handler + end + end + + # Nullify `Kemal::StaticFileHandler` + # + # Needed until the next release of Kemal after 1.7 + class Kemal::StaticFileHandler < HTTP::StaticFileHandler + def call(context : HTTP::Server::Context) + end + end + + {% skip_file %} +{% end %} + # Since systems have a limit on number of open files (`ulimit -a`), # we serve them from memory to avoid 'Too many open files' without needing # to modify ulimit. diff --git a/src/invidious.cr b/src/invidious.cr index 69f8a26c..358dc084 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -225,19 +225,25 @@ error 500 do |env, ex| error_template(500, ex) end -static_headers do |env| - env.response.headers.add("Cache-Control", "max-age=2629800") -end - # Init Kemal -public_folder "assets" - Kemal.config.powered_by_header = false add_handler FilteredCompressHandler.new add_handler APIHandler.new add_handler AuthHandler.new add_handler DenyFrame.new + +{% if compare_versions(Crystal::VERSION, "1.17.0") >= 0 %} + Kemal.config.serve_static = false + add_handler Invidious::HttpServer::StaticAssetsHandler.new("assets", directory_listing: false) +{% else %} + public_folder "assets" + + static_headers do |env| + env.response.headers.add("Cache-Control", "max-age=2629800") + end +{% end %} + add_context_storage_type(Array(String)) add_context_storage_type(Preferences) add_context_storage_type(Invidious::User) diff --git a/src/invidious/http_server/static_assets_handler.cr b/src/invidious/http_server/static_assets_handler.cr new file mode 100644 index 00000000..c6137775 --- /dev/null +++ b/src/invidious/http_server/static_assets_handler.cr @@ -0,0 +1,138 @@ +{% skip_file if compare_versions(Crystal::VERSION, "1.17.0") < 0 %} + +module Invidious::HttpServer + class StaticAssetsHandler < HTTP::StaticFileHandler + # In addition to storing the actual data of a file, it also implements the required + # getters needed for the object to imitate a `File::Stat` within `StaticFileHandler`. + # + # Since the `File::Stat` is created once in `#call` and then passed around to the + # rest of the class's methods, imitating the object allows us to only lookup + # the cache hash once for every request. + # + private record CachedFile, data : Bytes, size : Int64, modification_time : Time + + CACHE_LIMIT = 5_000_000 # 5MB + @@cached_files = {} of Path => CachedFile + + # A simplified version of `#call` for Invidious to improve performance. + # + # This is basically the same as what we inherited but just with the directory listing + # features stripped out. This removes some conditional checks and calls which improves + # performance slightly but otherwise is entirely unneeded. + # + # Really, all the cache feature actually needs is to override the much simplifier `file_info` + # method to return a `CachedFile` or `File::Stat` depending on whether the file is cached. + def call(context) : Nil + check_request_method!(context) || return + + request_path = request_path(context) + + check_request_path!(context, request_path) || return + + request_path = Path.posix(request_path) + expanded_path = request_path.expand("/") + + # The path normalization can be simplified to just this since + # we don't need to care about normalizing directory urls. + if request_path != expanded_path + redirect_to context, expanded_path + end + + file_path = @public_dir.join(expanded_path.to_kind(Path::Kind.native)) + + if cached_info = @@cached_files[file_path]? + return serve_file_with_cache(context, cached_info, file_path) + end + + file_info = File.info?(file_path) + + return call_next(context) unless file_info + + if file_info.file? + # Actually means to serve file *with cache headers* + # The actual logic for serving the file is done in `#serve_file` + serve_file_with_cache(context, file_info, file_path) + else # Not a normal file (FIFO/device/socket) + call_next(context) + end + end + + # Add "Cache-Control" header to the response + private def add_cache_headers(response_headers : HTTP::Headers, last_modified : Time) : Nil + super; response_headers["Cache-Control"] = "max-age=2629800" + end + + # Serves and caches the file at the given path. + # + # This is an override of `serve_file` to allow serving a file from memory, and to cache it + # it as needed. + private def serve_file(context : HTTP::Server::Context, file_info, file_path : Path, original_file_path : Path, last_modified : Time) + context.response.content_type = MIME.from_filename(original_file_path.to_s, "application/octet-stream") + + range_header = context.request.headers["Range"]? + + if !file_info.is_a? CachedFile + retrieve_bytes_from = IO::Memory.new + + File.open(file_path) do |file| + # We cannot cache partial data so we'll rewind and read from the start + if range_header + dispatch_serve(context, file, file_info, range_header) + IO.copy(file.rewind, retrieve_bytes_from) + else + context.response.output = IO::MultiWriter.new(context.response.output, retrieve_bytes_from, sync_close: true) + dispatch_serve(context, file, file_info, range_header) + end + end + + return flush_io_to_cache(retrieve_bytes_from, file_path, file_info) + else + return dispatch_serve(context, file_info.data, file_info, range_header) + end + end + + # Writes file data to the cache + private def flush_io_to_cache(io, file_path, file_info) + if @@cached_files.sum(&.[1].size) + (size = file_info.size) < CACHE_LIMIT + data_slice = io.to_slice + @@cached_files[file_path] = CachedFile.new(data_slice, file_info.size, file_info.modification_time) + end + end + + # Either send the file in full, or just fragments of it depending on the request + private def dispatch_serve(context, file, file_info, range_header) + if range_header + # an IO is needed for `serve_file_range` + file = file.is_a?(Bytes) ? IO::Memory.new(file, writeable: false) : file + serve_file_range(context, file, range_header, file_info) + else + context.response.headers["Accept-Ranges"] = "bytes" + serve_file_full(context, file, file_info) + end + end + + # Skips the stdlib logic for serving pre-gzipped files + private def serve_file_compressed(context : HTTP::Server::Context, file_info, file_path : Path, last_modified : Time) + serve_file(context, file_info, file_path, file_path, last_modified) + end + + # If we're serving the full file right away then there's no need for an IO at all. + private def serve_file_full(context : HTTP::Server::Context, file : Bytes, file_info) + context.response.status = :ok + context.response.content_length = file_info.size + context.response.write file + end + + # Serves segments of a file based on the `Range header` + # + # An override of `serve_file_range` to allow using a generic IO rather than a `File`. + # Literally the same code as what we inherited but just with the `file` argument's type + # being set to `IO` rather than `File` + # + # Can be removed once https://github.com/crystal-lang/crystal/issues/15817 is fixed. + private def serve_file_range(context : HTTP::Server::Context, file : IO, range_header : String, file_info) + # Paste in the body of inherited serve_file_range + {{@type.superclass.methods.select(&.name.==("serve_file_range"))[0].body}} + end + end +end