Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return files directly when serving JS files #349

Merged
merged 1 commit into from
May 6, 2019

Conversation

QuietMisdreavus
Copy link
Member

After my last docs.rs update over the weekend, i started noticing a handful of panic messages in the docs.rs server log. After hunting it down for a little bit i think i understand the culprit:

  • In serve all in-crate js files #329 i removed the search-index.js/aliases.js/source-files.js routes from the route table, instead handling them in the base rustdoc_redirector_handler. (Due to a bug in router (ultimately route-recognizer) i couldn't create a /:crate/:version/*.js route because it would be overridden by the /:crate/:version/:target route.) The intent was that routes like /crate/version/search-index.js would run through the branch i added in rustdoc_redirector_handler and get loaded up that way.
  • However, i had forgotten that other files were also running through this route. Top-level JavaScript files like main.js and storage.js are supposed to be caught by the SharedResourceHandler before the route table is even taken into account. However, one important file has been missing there for several rustdoc versions: settings.js. Since this is failing that handler, it's falling into the route table, where it also hits rustdoc_redirector_handler. Previously it would 404, but now...
  • After serve all in-crate js files #329 it would fall into the new branch and start hitting rustdoc_html_server_handler, where it would start hitting this code here. This is intended to cut off the crate name and version from the URL so that it can format the rest of the path into a path suitable for the files table in the database. However, the URL docs.rs/settings.js only has one segment in it. Calling req_path.remove(0) on it twice will cause it to clear out the path and attempt to index an empty Vec, causing a panic, and a 500 response.

This PR adds an extra check to rustdoc_redirector_handler to make sure that we try to serve top-level JS files directly instead of sending it through rustdoc_html_server_handler.

@QuietMisdreavus QuietMisdreavus merged commit 6b45c15 into rust-lang:master May 6, 2019
@QuietMisdreavus QuietMisdreavus deleted the web-panic branch May 6, 2019 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant