Fix local storyboard requests on Crystal >= 1.16.0 #5315
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
Note: extra care needs to be taken when merging alongside #5338 to also override the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 annoying than just changing a path. The "new" existence of the double slash means that
StaticFileHandlerwill 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
StaticFileHandlerto preserve query parameters when redirecting, and adds the/api/v1/storyboard/sb/*->/sb/i/*/redirect directly into Kemal'sExceptionHandlerfor now.In the future a dedicated handler should be created to properly handle http status code conditions on the API routes.