From 47d0d30a155653310a6fcd1b4e941fec370e4919 Mon Sep 17 00:00:00 2001 From: syeopite Date: Sat, 17 May 2025 22:09:47 -0700 Subject: [PATCH] Fix local storyboard requests on Crystal >= 1.16.0 Crystal 1.16.0 fixed the parsing of some weird-looking paths meaning that `//api/v1/storyboards/sb/i/...` will no longer get parsed into a request target of `/v1/storyboards/sb/i/...` within requests. But the fix is a bit more convoluted than just changing a path. The new presence of double slash means that `StaticFileHandler` will process and redirect the request to a normalized path with only a single slash. This redirect however does not preserve url query parameters which are needed to request storyboards from YouTube. Then there's the fact that the status code error handlers are disabled for the API routes, meaning that there is no handler to actually process the request with. This PR patches Kemal's `StaticFileHandler` to preserve query parameters when redirecting, and adds the `/api/v1/storyboard/sb/*` -> `/sb/i/*/` redirect directly into Kemal's `ExceptionHandler` for now. In the future a dedicated handler should be created to properly handle http status code conditions on the API routes. --- src/ext/kemal_static_file_handler.cr | 8 ++++++++ src/invidious/helpers/handlers.cr | 25 ++++++++++++++++++++++++- src/invidious/routes/errors.cr | 19 +++++++++++++++---- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/ext/kemal_static_file_handler.cr b/src/ext/kemal_static_file_handler.cr index eb068aeb..f3544c58 100644 --- a/src/ext/kemal_static_file_handler.cr +++ b/src/ext/kemal_static_file_handler.cr @@ -191,5 +191,13 @@ module Kemal end end end + + # See https://github.com/crystal-lang/crystal/issues/15788 + # + # URL fragments also aren't passed along but there isn't an easy way + # to retrieve that at the moment. + private def redirect_to(context, path) + context.response.redirect URI.new(path: URI.encode_path(path.to_s), query: context.request.query) + end end end diff --git a/src/invidious/helpers/handlers.cr b/src/invidious/helpers/handlers.cr index 13ea9fe9..4e53de6b 100644 --- a/src/invidious/helpers/handlers.cr +++ b/src/invidious/helpers/handlers.cr @@ -50,7 +50,30 @@ class Kemal::ExceptionHandler private def call_exception_with_status_code(context : HTTP::Server::Context, exception : Exception, status_code : Int32) return if context.response.closed? - return if exclude_match? context + + # Invidious excludes the status code error handlers from running on the api routes + # meaning that we are unable to redirect /api/v1/storyboards/sb/... to the right location + # within a 404 handler. As a quick fix we'll match it here. + # + # For future reference this is also why the API will always return a 200 status code + # even when a route could not be found. + # + # TODO: In the future there should be dedicated status code error handlers for the api. + # + if exclude_match?(context) + if status_code == 404 + # Only necessary on Crystal versions >= 1.16.0 + {% if compare_versions(Crystal::VERSION, "1.16.0") >= 0 %} + if HOST_URL.empty? && context.request.path.starts_with?("/api/v1/storyboards/sb") + return context.redirect "#{context.request.path[19..]}?#{context.params.query}", status_code: 302 + end + + return + {% end %} + end + + return + end if !Kemal.config.error_handlers.empty? && Kemal.config.error_handlers.has_key?(status_code) context.response.content_type = "text/html" unless context.response.headers.has_key?("Content-Type") diff --git a/src/invidious/routes/errors.cr b/src/invidious/routes/errors.cr index 1e9ab44e..5aea8c0e 100644 --- a/src/invidious/routes/errors.cr +++ b/src/invidious/routes/errors.cr @@ -1,9 +1,20 @@ module Invidious::Routes::ErrorRoutes def self.error_404(env) - # Workaround for #3117 - if HOST_URL.empty? && env.request.path.starts_with?("/v1/storyboards/sb") - return env.redirect "#{env.request.path[15..]}?#{env.params.query}" - end + # Workaround for #3117 on versions prior to 1.16.0 + # + # Crystal 1.16.0 fixed the parsing of some weird-looking paths + # meaning that `//api/v1/storyboards/sb/i/...` will no longer + # get parsed into a request target of `/v1/storyboards/sb/i/...` + # + # This also means that we won't be able to handle the logic here + # because status code error handles are disabled for the API routes + + # We only need to include this workaround on versions prior to 1.16.0 + {% if compare_versions(Crystal::VERSION, "1.16.0") < 0 %} + if HOST_URL.empty? && env.request.path.starts_with?("/v1/storyboards/sb") + return env.redirect "#{env.request.path[15..]}?#{env.params.query}" + end + {% end %} if md = env.request.path.match(/^\/(?([a-zA-Z0-9_-]{11})|(\w+))$/) item = md["id"]