Skip to content

Commit 5c5bedd

Browse files
GuillaumeGomezGuillaumeGomez
committed
Keep arguments in redirections
1 parent e7a97fd commit 5c5bedd

File tree

7 files changed

+97
-50
lines changed

7 files changed

+97
-50
lines changed

‎src/web/builds.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::{
1010
impl_axum_webpage,
1111
web::{
1212
MetaData, ReqVersion,
13-
error::AxumResult,
13+
error::{AxumResult, EscapedURI},
1414
extractors::{DbConnection, Path},
1515
filters, match_version,
1616
page::templates::{RenderRegular, RenderSolid},
@@ -69,7 +69,7 @@ pub(crate) async fn build_list_handler(
6969
.assume_exact_name()?
7070
.into_canonical_req_version_or_else(|version| {
7171
AxumNope::Redirect(
72-
format!("/crate/{name}/{version}/builds"),
72+
EscapedURI::new(&format!("/crate/{name}/{version}/builds"), None),
7373
CachePolicy::ForeverInCdn,
7474
)
7575
})?
@@ -93,7 +93,7 @@ pub(crate) async fn build_list_json_handler(
9393
.assume_exact_name()?
9494
.into_canonical_req_version_or_else(|version| {
9595
AxumNope::Redirect(
96-
format!("/crate/{name}/{version}/builds.json"),
96+
EscapedURI::new(&format!("/crate/{name}/{version}/builds.json"), None),
9797
CachePolicy::ForeverInCdn,
9898
)
9999
})?

‎src/web/crate_details.rs

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ use crate::{
1010
web::{
1111
MatchedRelease, ReqVersion,
1212
cache::CachePolicy,
13-
encode_url_path,
14-
error::{AxumNope, AxumResult},
13+
error::{AxumNope, AxumResult, EscapedURI},
1514
extractors::{DbConnection, Path},
1615
page::templates::{RenderRegular, RenderSolid, filters},
1716
rustdoc::RustdocHtmlParams,
@@ -475,7 +474,10 @@ pub(crate) async fn crate_details_handler(
475474
) -> AxumResult<AxumResponse> {
476475
let req_version = params.version.ok_or_else(|| {
477476
AxumNope::Redirect(
478-
format!("/crate/{}/{}", &params.name, ReqVersion::Latest),
477+
EscapedURI::new(
478+
&format!("/crate/{}/{}", &params.name, ReqVersion::Latest),
479+
None,
480+
),
479481
CachePolicy::ForeverInCdn,
480482
)
481483
})?;
@@ -485,7 +487,7 @@ pub(crate) async fn crate_details_handler(
485487
.assume_exact_name()?
486488
.into_canonical_req_version_or_else(|version| {
487489
AxumNope::Redirect(
488-
format!("/crate/{}/{}", &params.name, version),
490+
EscapedURI::new(&format!("/crate/{}/{}", &params.name, version), None),
489491
CachePolicy::ForeverInCdn,
490492
)
491493
})?;
@@ -694,23 +696,29 @@ pub(crate) async fn get_all_platforms_inner(
694696
.await?
695697
.into_exactly_named_or_else(|corrected_name, req_version| {
696698
AxumNope::Redirect(
697-
encode_url_path(&format!(
698-
"/platforms/{}/{}/{}",
699-
corrected_name,
700-
req_version,
701-
req_path.join("/")
702-
)),
699+
EscapedURI::new(
700+
&format!(
701+
"/platforms/{}/{}/{}",
702+
corrected_name,
703+
req_version,
704+
req_path.join("/")
705+
),
706+
None,
707+
),
703708
CachePolicy::NoCaching,
704709
)
705710
})?
706711
.into_canonical_req_version_or_else(|version| {
707712
AxumNope::Redirect(
708-
encode_url_path(&format!(
709-
"/platforms/{}/{}/{}",
710-
&params.name,
711-
version,
712-
req_path.join("/")
713-
)),
713+
EscapedURI::new(
714+
&format!(
715+
"/platforms/{}/{}/{}",
716+
&params.name,
717+
version,
718+
req_path.join("/")
719+
),
720+
None,
721+
),
714722
CachePolicy::ForeverInCdn,
715723
)
716724
})?;

‎src/web/error.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,24 @@ use tracing::error;
1414

1515
use super::AxumErrorPage;
1616

17+
#[derive(Debug)]
18+
pub struct EscapedURI(String);
19+
20+
impl EscapedURI {
21+
pub fn new(path: &str, query: Option<&str>) -> Self {
22+
let mut path = encode_url_path(path);
23+
if let Some(query) = query {
24+
path.push('?');
25+
path.push_str(query);
26+
}
27+
Self(path)
28+
}
29+
30+
pub fn as_str(&self) -> &str {
31+
self.0.as_str()
32+
}
33+
}
34+
1735
#[derive(Debug, thiserror::Error)]
1836
pub enum AxumNope {
1937
#[error("Requested resource not found")]
@@ -35,7 +53,7 @@ pub enum AxumNope {
3553
#[error("bad request")]
3654
BadRequest(anyhow::Error),
3755
#[error("redirect")]
38-
Redirect(String, CachePolicy),
56+
Redirect(EscapedURI, CachePolicy),
3957
}
4058

4159
// FUTURE: Ideally, the split between the 3 kinds of responses would
@@ -118,8 +136,8 @@ struct ErrorInfo {
118136
pub status: StatusCode,
119137
}
120138

121-
fn redirect_with_policy(target: String, cache_policy: CachePolicy) -> AxumResponse {
122-
match super::axum_cached_redirect(encode_url_path(&target), cache_policy) {
139+
fn redirect_with_policy(target: EscapedURI, cache_policy: CachePolicy) -> AxumResponse {
140+
match super::axum_cached_redirect(target.0, cache_policy) {
123141
Ok(response) => response.into_response(),
124142
Err(err) => AxumNope::InternalError(err).into_response(),
125143
}
@@ -215,16 +233,18 @@ pub(crate) type JsonAxumResult<T> = Result<T, JsonAxumNope>;
215233

216234
#[cfg(test)]
217235
mod tests {
218-
use super::{AxumNope, IntoResponse};
236+
use super::{AxumNope, EscapedURI, IntoResponse};
219237
use crate::test::{AxumResponseTestExt, AxumRouterTestExt, async_wrapper};
220238
use crate::web::cache::CachePolicy;
221239
use kuchikiki::traits::TendrilSink;
222240

223241
#[test]
224242
fn test_redirect_error_encodes_url_path() {
225-
let response =
226-
AxumNope::Redirect("/something>".into(), CachePolicy::ForeverInCdnAndBrowser)
227-
.into_response();
243+
let response = AxumNope::Redirect(
244+
EscapedURI::new("/something>", None),
245+
CachePolicy::ForeverInCdnAndBrowser,
246+
)
247+
.into_response();
228248

229249
assert_eq!(response.status(), 302);
230250
assert_eq!(response.headers().get("Location").unwrap(), "/something%3E");

‎src/web/features.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
web::{
55
MetaData, ReqVersion,
66
cache::CachePolicy,
7-
error::{AxumNope, AxumResult},
7+
error::{AxumNope, AxumResult, EscapedURI},
88
extractors::{DbConnection, Path},
99
filters,
1010
headers::CanonicalUrl,
@@ -145,7 +145,7 @@ pub(crate) async fn build_features_handler(
145145
.assume_exact_name()?
146146
.into_canonical_req_version_or_else(|version| {
147147
AxumNope::Redirect(
148-
format!("/crate/{}/{}/features", &name, version),
148+
EscapedURI::new(&format!("/crate/{}/{}/features", &name, version), None),
149149
CachePolicy::ForeverInCdn,
150150
)
151151
})?

‎src/web/rustdoc.rs

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::{
1111
crate_details::CrateDetails,
1212
csp::Csp,
1313
encode_url_path,
14-
error::{AxumNope, AxumResult},
14+
error::{AxumNope, AxumResult, EscapedURI},
1515
extractors::{DbConnection, Path},
1616
file::File,
1717
match_version,
@@ -362,18 +362,26 @@ pub(crate) async fn rustdoc_html_server_handler(
362362
// Pages generated by Rustdoc are not ready to be served with a CSP yet.
363363
csp.suppress(true);
364364

365+
fn get_query_and_fragment(uri: &Uri) -> Option<&str> {
366+
// We cannot extract the fragment (`#`) with current `Uri` API so forced to do ugly
367+
// things like that...
368+
uri.path_and_query()
369+
.and_then(|path_and_query| path_and_query.as_str().splitn(2, '?').nth(1))
370+
}
371+
365372
// Convenience function to allow for easy redirection
366373
#[instrument]
367374
fn redirect(
368375
name: &str,
369376
vers: &Version,
370377
path: &[&str],
371378
cache_policy: CachePolicy,
379+
uri: &Uri,
372380
) -> AxumResult<AxumResponse> {
373381
trace!("redirect");
374-
// Format and parse the redirect url
382+
let query = get_query_and_fragment(&uri);
375383
Ok(axum_cached_redirect(
376-
encode_url_path(&format!("/{}/{}/{}", name, vers, path.join("/"))),
384+
EscapedURI::new(&format!("/{}/{}/{}", name, vers, path.join("/")), query).as_str(),
377385
cache_policy,
378386
)?
379387
.into_response())
@@ -391,24 +399,21 @@ pub(crate) async fn rustdoc_html_server_handler(
391399
let matched_release = match_version(&mut conn, &params.name, &params.version)
392400
.await?
393401
.into_exactly_named_or_else(|corrected_name, req_version| {
402+
let query = get_query_and_fragment(&uri);
394403
AxumNope::Redirect(
395-
encode_url_path(&format!(
396-
"/{}/{}/{}",
397-
corrected_name,
398-
req_version,
399-
req_path.join("/")
400-
)),
404+
EscapedURI::new(
405+
&format!("/{}/{}/{}", corrected_name, req_version, req_path.join("/")),
406+
query,
407+
),
401408
CachePolicy::NoCaching,
402409
)
403410
})?
404411
.into_canonical_req_version_or_else(|version| {
405412
AxumNope::Redirect(
406-
encode_url_path(&format!(
407-
"/{}/{}/{}",
408-
&params.name,
409-
version,
410-
req_path.join("/")
411-
)),
413+
EscapedURI::new(
414+
&format!("/{}/{}/{}", &params.name, version, req_path.join("/")),
415+
None,
416+
),
412417
CachePolicy::ForeverInCdn,
413418
)
414419
})?;
@@ -439,6 +444,7 @@ pub(crate) async fn rustdoc_html_server_handler(
439444
&krate.version,
440445
&req_path[1..],
441446
CachePolicy::ForeverInCdn,
447+
&uri,
442448
);
443449
}
444450

@@ -494,6 +500,7 @@ pub(crate) async fn rustdoc_html_server_handler(
494500
&krate.version,
495501
&req_path,
496502
CachePolicy::ForeverInCdn,
503+
&uri,
497504
);
498505
}
499506
}
@@ -2004,6 +2011,12 @@ mod test {
20042011
"/foo-ab/0.0.1/foo_ab/",
20052012
)
20062013
.await?;
2014+
// `-` becomes `_` but we keep the query arguments.
2015+
web.assert_redirect_unchecked(
2016+
"/foo-ab/0.0.1/foo_ab/?search=a",
2017+
"/foo_ab/0.0.1/foo_ab/?search=a",
2018+
)
2019+
.await?;
20072020
Ok(())
20082021
})
20092022
}

‎src/web/source.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
web::{
88
MetaData, ReqVersion,
99
cache::CachePolicy,
10-
error::AxumNope,
10+
error::{AxumNope, EscapedURI},
1111
extractors::Path,
1212
file::File as DbFile,
1313
headers::CanonicalUrl,
@@ -203,16 +203,22 @@ pub(crate) async fn source_browser_handler(
203203
.await?
204204
.into_exactly_named_or_else(|corrected_name, req_version| {
205205
AxumNope::Redirect(
206-
format!(
207-
"/crate/{corrected_name}/{req_version}/source/{}",
208-
params.path
206+
EscapedURI::new(
207+
&format!(
208+
"/crate/{corrected_name}/{req_version}/source/{}",
209+
params.path
210+
),
211+
None,
209212
),
210213
CachePolicy::NoCaching,
211214
)
212215
})?
213216
.into_canonical_req_version_or_else(|version| {
214217
AxumNope::Redirect(
215-
format!("/crate/{}/{version}/source/{}", params.name, params.path),
218+
EscapedURI::new(
219+
&format!("/crate/{}/{version}/source/{}", params.name, params.path),
220+
None,
221+
),
216222
CachePolicy::ForeverInCdn,
217223
)
218224
})?

‎src/web/status.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::{cache::CachePolicy, error::AxumNope};
22
use crate::web::{
33
ReqVersion,
4-
error::AxumResult,
4+
error::{AxumResult, EscapedURI},
55
extractors::{DbConnection, Path},
66
match_version,
77
};
@@ -28,7 +28,7 @@ pub(crate) async fn status_handler(
2828
let version = matched_release
2929
.into_canonical_req_version_or_else(|version| {
3030
AxumNope::Redirect(
31-
format!("/crate/{name}/{version}/status.json"),
31+
EscapedURI::new(&format!("/crate/{name}/{version}/status.json"), None),
3232
CachePolicy::NoCaching,
3333
)
3434
})?

0 commit comments

Comments
 (0)