Skip to content

Commit

Permalink
Fix overlapping route rewrites (#27)
Browse files Browse the repository at this point in the history 
Case is, target service on the remote side is a tunnel.
If the path gets rewritten once remotely, it can throw off finding the
right service in the local server.

Solution: an extra header linkup-destination set on every hop. If the
destination has been calculated once, we reuse it on later hops.
  • Loading branch information
@ostenbom
ostenbom authored Sep 8, 2023
1 parent cc69305 commit 7d699c1
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 52 deletions.
40 changes: 20 additions & 20 deletions linkup-cli/src/local_server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,17 @@ async fn linkup_ws_request_handler(
}
};

let destination_url = match get_target_url(url.clone(), headers.clone(), &config, &session_name)
{
Some(result) => result,
None => {
return HttpResponse::NotFound()
.append_header(header::ContentType::plaintext())
.body("Not target url for request - local server")
}
};
let (dest_service_name, destination_url) =
match get_target_service(url.clone(), headers.clone(), &config, &session_name) {
Some(result) => result,
None => {
return HttpResponse::NotFound()
.append_header(header::ContentType::plaintext())
.body("Not target url for request - local server")
}
};

let extra_headers = get_additional_headers(url, &headers, &session_name);
let extra_headers = get_additional_headers(url, &headers, &session_name, &dest_service_name);

// Proxy the request using the destination_url and the merged headers
let client = reqwest::Client::new();
Expand Down Expand Up @@ -191,17 +191,17 @@ async fn linkup_request_handler(
}
};

let destination_url = match get_target_url(url.clone(), headers.clone(), &config, &session_name)
{
Some(result) => result,
None => {
return HttpResponse::NotFound()
.append_header(header::ContentType::plaintext())
.body("Not target url for request - local server")
}
};
let (dest_service_name, destination_url) =
match get_target_service(url.clone(), headers.clone(), &config, &session_name) {
Some(result) => result,
None => {
return HttpResponse::NotFound()
.append_header(header::ContentType::plaintext())
.body("Not target url for request - local server")
}
};

let extra_headers = get_additional_headers(url, &headers, &session_name);
let extra_headers = get_additional_headers(url, &headers, &session_name, &dest_service_name);

// Proxy the request using the destination_url and the merged headers
let client = reqwest::Client::new();
Expand Down
143 changes: 124 additions & 19 deletions linkup/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub fn get_additional_headers(
url: String,
headers: &HashMap<String, String>,
session_name: &str,
destination_service_name: &str,
) -> HashMap<String, String> {
let mut additional_headers = HashMap::new();

Expand All @@ -64,7 +65,7 @@ pub fn get_additional_headers(
}

let tracestate = headers.get("tracestate");
let linkup_session = format!("linkup-session={}", session_name);
let linkup_session = format!("linkup-session={}", session_name,);
match tracestate {
Some(ts) if !ts.contains(&linkup_session) => {
let new_tracestate = format!("{},{}", ts, linkup_session);
Expand All @@ -77,6 +78,13 @@ pub fn get_additional_headers(
_ => {}
}

if !headers.contains_key("linkup-destination") {
additional_headers.insert(
"linkup-destination".to_string(),
destination_service_name.to_string(),
);
}

if !headers.contains_key("X-Forwarded-Host") {
additional_headers.insert(
"X-Forwarded-Host".to_string(),
Expand All @@ -101,23 +109,34 @@ pub fn additional_response_headers() -> HashMap<String, String> {
headers
}

// Returns a url for the destination service and the service name, if the request could be served by the config
pub fn get_target_url(
// Returns a (name, url) pair for the destination service, if the request could be served by the config
pub fn get_target_service(
url: String,
headers: HashMap<String, String>,
config: &Session,
session_name: &str,
) -> Option<String> {
) -> Option<(String, String)> {
let target = Url::parse(&url).unwrap();
let path = target.path();

// If there was a destination created in a previous linkup, we don't want to
// re-do path rewrites, so we use the destination service.
if let Some(destination_service) = headers.get("linkup-destination") {
if let Some(service) = config.services.get(destination_service) {
let target = redirect(target.clone(), &service.origin, Some(path.to_string()));
return Some((destination_service.to_string(), String::from(target)));
}
}

let url_target = config.domains.get(&get_target_domain(&url, session_name));

// Forwarded hosts persist over the tunnel
let forwarded_host_target = config.domains.get(
headers
.get("x-forwarded-host")
.unwrap_or(&"does-not-exist".to_string()),
headers.get("x-forwarded-host").unwrap_or(
headers
.get("X-Forwarded-Host")
.unwrap_or(&"does-not-exist".to_string()),
),
);

// This is more for e2e tests to work
Expand Down Expand Up @@ -171,7 +190,7 @@ pub fn get_target_url(
}

let target = redirect(target, &service.origin, Some(new_path));
return Some(String::from(target));
return Some((service_name, String::from(target)));
}
}

Expand Down Expand Up @@ -265,6 +284,12 @@ mod tests {
},
{
"name": "backend",
"rewrites": [
{
"source": "/api/v2/(.*)",
"target": "/$1"
}
],
"location": "http://localhost:8001/"
}
],
Expand All @@ -276,6 +301,10 @@ mod tests {
{
"path": "/api/v1/.*",
"service": "backend"
},
{
"path": "/api/v2/.*",
"service": "backend"
}
]
},
Expand Down Expand Up @@ -347,11 +376,13 @@ mod tests {
#[test]
fn test_get_additional_headers() {
let session_name = String::from("tiny-cow");
let destination_service_name = String::from("frontend");
let headers = HashMap::new();
let add_headers = get_additional_headers(
"https://tiny-cow.example.com/abc-xyz".to_string(),
&headers,
&session_name,
&destination_service_name,
);

assert_eq!(add_headers.get("traceparent").unwrap().len(), 55);
Expand All @@ -360,6 +391,7 @@ mod tests {
"linkup-session=tiny-cow"
);
assert_eq!(add_headers.get("X-Forwarded-Host").unwrap(), "example.com");
assert_eq!(add_headers.get("linkup-destination").unwrap(), "frontend");

let mut already_headers: HashMap<String, String> = HashMap::new();
already_headers.insert("traceparent".to_string(), "anything".to_string());
Expand All @@ -368,15 +400,18 @@ mod tests {
"linkup-session=tiny-cow".to_string(),
);
already_headers.insert("X-Forwarded-Host".to_string(), "example.com".to_string());
already_headers.insert("linkup-destination".to_string(), "frontend".to_string());
let add_headers = get_additional_headers(
"https://abc.some-tunnel.com/abc-xyz".to_string(),
&already_headers,
&session_name,
&destination_service_name,
);

assert!(add_headers.get("traceparent").is_none());
assert!(add_headers.get("X-Forwarded-Host").is_none());
assert!(add_headers.get("tracestate").is_none());
assert!(add_headers.get("X-Forwarded-Host").is_none());
assert!(add_headers.get("linkup-destination").is_none());

let mut already_headers_two: HashMap<String, String> = HashMap::new();
already_headers_two.insert("traceparent".to_string(), "anything".to_string());
Expand All @@ -386,6 +421,7 @@ mod tests {
"https://abc.some-tunnel.com/abc-xyz".to_string(),
&already_headers_two,
&session_name,
&destination_service_name,
);

assert!(add_headers.get("traceparent").is_none());
Expand Down Expand Up @@ -426,58 +462,127 @@ mod tests {

// Standard named subdomain
assert_eq!(
get_target_url(
get_target_service(
format!("http://{}.example.com/?a=b", &name),
HashMap::new(),
&config,
&name
)
.unwrap(),
"http://localhost:8000/?a=b".to_string(),
(
"frontend".to_string(),
"http://localhost:8000/?a=b".to_string()
),
);
// With path
assert_eq!(
get_target_url(
get_target_service(
format!("http://{}.example.com/a/b/c/?a=b", &name),
HashMap::new(),
&config,
&name
)
.unwrap(),
"http://localhost:8000/a/b/c/?a=b".to_string(),
(
"frontend".to_string(),
"http://localhost:8000/a/b/c/?a=b".to_string()
),
);
// Test rewrites
assert_eq!(
get_target_url(
get_target_service(
format!("http://{}.example.com/foo/b/c/?a=b", &name),
HashMap::new(),
&config,
&name
)
.unwrap(),
"http://localhost:8000/bar/b/c/?a=b".to_string(),
(
"frontend".to_string(),
"http://localhost:8000/bar/b/c/?a=b".to_string()
),
);
// Test domain routes
assert_eq!(
get_target_url(
get_target_service(
format!("http://{}.example.com/api/v1/?a=b", &name),
HashMap::new(),
&config,
&name
)
.unwrap(),
"http://localhost:8001/api/v1/?a=b".to_string(),
(
"backend".to_string(),
"http://localhost:8001/api/v1/?a=b".to_string()
)
);
// Test no named subdomain
assert_eq!(
get_target_url(
get_target_service(
"http://api.example.com/api/v1/?a=b".to_string(),
HashMap::new(),
&config,
&name
)
.unwrap(),
"http://localhost:8001/api/v1/?a=b".to_string(),
(
"backend".to_string(),
"http://localhost:8001/api/v1/?a=b".to_string()
)
);
}

#[tokio::test]
async fn test_repeatable_rewritten_routes() {
let sessions = SessionAllocator::new(Arc::new(MemoryStringStore::new()));

let input_config_value: serde_json::Value = serde_json::from_str(CONF_STR).unwrap();
let input_config: Session = input_config_value.try_into().unwrap();

let name = sessions
.store_session(input_config, NameKind::Animal, "".to_string())
.await
.unwrap();

let (name, config) = sessions
.get_request_session(format!("{}.example.com", name), HashMap::new())
.await
.unwrap();

// Case is, target service on the remote side is a tunnel.
// If the path gets rewritten once remotely, it can throw off finding
// the right service in the local server

let (service, service_url) = get_target_service(
"http://example.com/api/v2/user".to_string(),
HashMap::new(),
&config,
&name,
)
.unwrap();

// First request as normal
assert_eq!(service, "backend");
assert_eq!(service_url, "http://localhost:8001/user");

let extra_headers = get_additional_headers(
"http://example.com/api/v2/user".to_string(),
&HashMap::new(),
&name,
&service,
);

let (service, service_url) = get_target_service(
"http://localhost:8001/user".to_string(),
extra_headers,
&config,
&name,
)
.unwrap();

// Second request should have the same outcome
// The secret sauce should be in the extra headers that have been propogated
assert_eq!(service, "backend");
assert_eq!(service_url, "http://localhost:8001/user");
}
}
12 changes: 6 additions & 6 deletions worker/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ async fn linkup_request_handler(mut req: Request, sessions: SessionAllocator) ->
Err(_) => return plaintext_error("Bad or missing request body", 400),
};

let destination_url = match get_target_url(url.clone(), headers.clone(), &config, &session_name)
{
Some(result) => result,
None => return plaintext_error("No target URL for request", 422),
};
let (dest_service_name, destination_url) =
match get_target_service(url.clone(), headers.clone(), &config, &session_name) {
Some(result) => result,
None => return plaintext_error("No target URL for request", 422),
};

let extra_headers = get_additional_headers(url, &headers, &session_name);
let extra_headers = get_additional_headers(url, &headers, &session_name, &dest_service_name);

let method = match convert_cf_method_to_reqwest(&req.method()) {
Ok(method) => method,
Expand Down
Loading

0 comments on commit 7d699c1

Please sign in to comment.