Skip to content

Refactor static file handling #1090

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 13, 2020
Merged

Refactor static file handling #1090

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b50dd13
Refactor static file handling to only use a single handler
Nemo157 Oct 12, 2020
02aee32
Remove staticfile dependency
Nemo157 Oct 12, 2020
67641d1
Add metrics tracking DatabaseFileHandler usage
Nemo157 Oct 12, 2020
c4eaa7b
Don't move the sitemap
Nemo157 Oct 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
use git2::Repository;
use std::{
env,
error::Error,
fs::{self, File},
io::Write,
path::Path,
};
use std::{env, error::Error, fs::File, io::Write, path::Path};

fn main() {
// Don't rerun anytime a single change is made
Expand All @@ -15,8 +9,6 @@ fn main() {
println!("cargo:rerun-if-changed=templates/style/_vars.scss");
println!("cargo:rerun-if-changed=templates/style/_utils.scss");
println!("cargo:rerun-if-changed=templates/style/_navbar.scss");
println!("cargo:rerun-if-changed=templates/menu.js");
println!("cargo:rerun-if-changed=templates/index.js");
println!("cargo:rerun-if-changed=vendor/");
// TODO: are these right?
println!("cargo:rerun-if-changed=.git/HEAD");
Expand All @@ -26,7 +18,6 @@ fn main() {
if let Err(sass_err) = compile_sass() {
panic!("Error compiling sass: {}", sass_err);
}
copy_js();
}

fn write_git_version() {
Expand Down Expand Up @@ -95,12 +86,3 @@ fn compile_sass() -> Result<(), Box<dyn Error>> {

Ok(())
}

fn copy_js() {
["menu.js", "index.js"].iter().for_each(|path| {
let source_path =
Path::new(&env::var("CARGO_MANIFEST_DIR").unwrap()).join(format!("templates/{}", path));
let dest_path = Path::new(&env::var("OUT_DIR").unwrap()).join(path);
fs::copy(&source_path, &dest_path).expect("Copy JavaScript file to target");
});
}
4 changes: 1 addition & 3 deletions dockerfiles/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ RUN touch build.rs
COPY src src/
RUN find src -name "*.rs" -exec touch {} \;
COPY templates/style templates/style
COPY templates/index.js templates/
COPY templates/menu.js templates/
COPY vendor vendor/

RUN cargo build --release
Expand All @@ -76,7 +74,7 @@ RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y \
RUN mkdir -p /opt/docsrs/prefix

COPY --from=build /build/target/release/cratesfyi /usr/local/bin
COPY static /opt/docsrs/prefix/public_html
COPY static /opt/docsrs/static
COPY templates /opt/docsrs/templates
COPY dockerfiles/entrypoint.sh /opt/docsrs/
COPY vendor /opt/docsrs/vendor
Expand Down
10 changes: 5 additions & 5 deletions src/web/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ mod tests {
.next()
.unwrap()
.text_contents(),
"The requested resource does not exist",
"The requested crate does not exist",
);

Ok(())
Expand All @@ -170,7 +170,7 @@ mod tests {
.next()
.unwrap()
.text_contents(),
"The requested resource does not exist",
"The requested crate does not exist",
);

Ok(())
Expand All @@ -190,7 +190,7 @@ mod tests {
.next()
.unwrap()
.text_contents(),
"The requested resource does not exist",
"The requested crate does not exist",
);

Ok(())
Expand All @@ -209,7 +209,7 @@ mod tests {
.next()
.unwrap()
.text_contents(),
"The requested resource does not exist",
"The requested crate does not exist",
);

Ok(())
Expand All @@ -232,7 +232,7 @@ mod tests {
.next()
.unwrap()
.text_contents(),
"The requested resource does not exist",
"The requested crate does not exist",
);

Ok(())
Expand Down
6 changes: 3 additions & 3 deletions src/web/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ mod tests {
("/crate/rcc/0.0.0", "/crate/:name/:version"),
("/-/static/index.js", "static resource"),
("/-/static/menu.js", "static resource"),
("/opensearch.xml", "static resource"),
("/-/static/opensearch.xml", "static resource"),
("/releases", "/releases"),
("/releases/feed", "static resource"),
("/releases/queue", "/releases/queue"),
Expand All @@ -129,8 +129,8 @@ mod tests {
"/releases/recent-failures/:page",
),
("/releases/recent/1", "/releases/recent/:page"),
("/robots.txt", "static resource"),
("/sitemap.xml", "static resource"),
("/-/static/robots.txt", "static resource"),
("/-/sitemap.xml", "static resource"),
("/-/static/style.css", "static resource"),
("/-/static/vendored.css", "static resource"),
];
Expand Down
31 changes: 2 additions & 29 deletions src/web/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ use extensions::InjectExtensions;
use failure::Error;
use iron::{
self,
headers::{CacheControl, CacheDirective, ContentType, Expires, HttpDate},
headers::{Expires, HttpDate},
modifiers::Redirect,
status,
status::Status,
Expand All @@ -107,20 +107,17 @@ use postgres::Client;
use router::NoRoute;
use semver::{Version, VersionReq};
use serde::Serialize;
use staticfile::Static;
use std::{borrow::Cow, env, fmt, net::SocketAddr, path::PathBuf, sync::Arc, time::Duration};
use std::{borrow::Cow, fmt, net::SocketAddr, sync::Arc};

/// Duration of static files for staticfile and DatabaseFileHandler (in seconds)
const STATIC_FILE_CACHE_DURATION: u64 = 60 * 60 * 24 * 30 * 12; // 12 months
const OPENSEARCH_XML: &[u8] = include_bytes!("opensearch.xml");

const DEFAULT_BIND: &str = "0.0.0.0:3000";

struct CratesfyiHandler {
shared_resource_handler: Box<dyn Handler>,
router_handler: Box<dyn Handler>,
database_file_handler: Box<dyn Handler>,
static_handler: Box<dyn Handler>,
inject_extensions: InjectExtensions,
}

Expand All @@ -144,13 +141,6 @@ impl CratesfyiHandler {
let shared_resources =
Self::chain(inject_extensions.clone(), rustdoc::SharedResourceHandler);
let router_chain = Self::chain(inject_extensions.clone(), routes.iron_router());
let prefix = PathBuf::from(
env::var("CRATESFYI_PREFIX")
.expect("the CRATESFYI_PREFIX environment variable is not set"),
)
.join("public_html");
let static_handler =
Static::new(prefix).cache(Duration::from_secs(STATIC_FILE_CACHE_DURATION));

Ok(CratesfyiHandler {
shared_resource_handler: Box::new(shared_resources),
Expand All @@ -159,7 +149,6 @@ impl CratesfyiHandler {
blacklisted_prefixes,
Box::new(file::DatabaseFileHandler),
)),
static_handler: Box::new(static_handler),
inject_extensions,
})
}
Expand All @@ -185,7 +174,6 @@ impl Handler for CratesfyiHandler {
.handle(req)
.or_else(|e| if_404(e, || self.router_handler.handle(req)))
.or_else(|e| if_404(e, || self.database_file_handler.handle(req)))
.or_else(|e| if_404(e, || self.static_handler.handle(req)))
.or_else(|e| {
let err = if let Some(err) = e.error.downcast::<error::Nope>() {
*err
Expand Down Expand Up @@ -518,21 +506,6 @@ fn redirect_base(req: &Request) -> String {
}
}

fn opensearch_xml_handler(_: &mut Request) -> IronResult<Response> {
let mut response = Response::with((status::Ok, OPENSEARCH_XML));
let cache = vec![
CacheDirective::Public,
CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32),
];
response.headers.set(ContentType(
"application/opensearchdescription+xml".parse().unwrap(),
));

response.headers.set(CacheControl(cache));

Ok(response)
}

/// MetaData used in header
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
pub(crate) struct MetaData {
Expand Down
57 changes: 53 additions & 4 deletions src/web/routes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,18 @@ pub(super) const DOC_RUST_LANG_ORG_REDIRECTS: &[&str] =
pub(super) fn build_routes() -> Routes {
let mut routes = Routes::new();

routes.static_resource("/robots.txt", super::sitemap::robots_txt_handler);
routes.static_resource("/sitemap.xml", super::sitemap::sitemap_handler);
routes.static_resource("/opensearch.xml", super::opensearch_xml_handler);
routes.static_resource("/robots.txt", PermanentRedirect("/-/static/robots.txt"));
routes.static_resource("/favicon.ico", PermanentRedirect("/-/static/favicon.ico"));

// These should not need to be served from the root as we reference the inner path in links,
// but clients might have cached the url and need to update it.
routes.static_resource(
"/opensearch.xml",
PermanentRedirect("/-/static/opensearch.xml"),
);
routes.static_resource("/sitemap.xml", PermanentRedirect("/-/sitemap.xml"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sitemap has to live at the top level:

A sitemap can be posted anywhere on your site, but a sitemap affects only descendants of the parent directory. Therefore, a sitemap posted at the site root can affect all files on the site, which is where we recommend posting your sitemaps.

https://support.google.com/webmasters/answer/183668?hl=en

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that was one I was wondering about, I'll change this back and put in a comment.


routes.static_resource("/-/sitemap.xml", super::sitemap::sitemap_handler);
routes.static_resource("/-/static/:file", super::statics::static_handler);

routes.internal_page("/", super::releases::home_page);
Expand Down Expand Up @@ -257,7 +266,21 @@ impl Handler for SimpleRedirect {
(self.url_mangler)(&mut url);
Ok(iron::Response::with((
iron::status::Found,
iron::modifiers::Redirect(iron::Url::parse(&url.to_string()).unwrap()),
iron::modifiers::Redirect(iron::Url::from_generic_url(url).unwrap()),
)))
}
}

#[derive(Copy, Clone)]
struct PermanentRedirect(&'static str);

impl Handler for PermanentRedirect {
fn handle(&self, req: &mut iron::Request) -> iron::IronResult<iron::Response> {
let mut url: iron::url::Url = req.url.clone().into();
url.set_path(self.0);
Ok(iron::Response::with((
iron::status::MovedPermanently,
iron::modifiers::Redirect(iron::Url::from_generic_url(url).unwrap()),
)))
}
}
Expand Down Expand Up @@ -299,3 +322,29 @@ fn calculate_id(pattern: &str) -> String {
.map(|c| if c.is_alphanumeric() { c } else { '_' })
.collect()
}

#[cfg(test)]
mod tests {
use crate::test::*;

#[test]
fn test_root_redirects() {
wrapper(|env| {
// These are "well-known" resources that must be served from the root
assert_redirect("/favicon.ico", "/-/static/favicon.ico", env.frontend())?;
assert_redirect("/robots.txt", "/-/static/robots.txt", env.frontend())?;

// These have previously been served with a url pointing to the root, it may be
// plausible to remove the redirects in the future, but for now we need to keep serving
// them.
assert_redirect(
"/opensearch.xml",
"/-/static/opensearch.xml",
env.frontend(),
)?;
assert_redirect("/sitemap.xml", "/-/sitemap.xml", env.frontend())?;

Ok(())
});
}
}
9 changes: 1 addition & 8 deletions src/web/sitemap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use chrono::{DateTime, NaiveDateTime, Utc};
use iron::{
headers::ContentType,
mime::{Mime, SubLevel, TopLevel},
status, IronResult, Request, Response,
IronResult, Request, Response,
};
use serde::Serialize;
use serde_json::Value;
Expand Down Expand Up @@ -48,13 +48,6 @@ pub fn sitemap_handler(req: &mut Request) -> IronResult<Response> {
SitemapXml { releases }.into_response(req)
}

pub fn robots_txt_handler(_: &mut Request) -> IronResult<Response> {
let mut resp = Response::with((status::Ok, "Sitemap: https://docs.rs/sitemap.xml"));
resp.headers.set(ContentType::plaintext());

Ok(resp)
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
struct AboutBuilds {
/// The current version of rustc that docs.rs is using to build crates
Expand Down
Loading