Skip to content

Commit

Permalink
Merge pull request #8904 from jolicode/project-configuration-domains
Browse files Browse the repository at this point in the history
Do not consider unknown domain as a redirection loop
  • Loading branch information
joelwurtz authored May 16, 2024
2 parents 2882c91 + 692a7b1 commit 94b1cec
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 12 deletions.
21 changes: 18 additions & 3 deletions src/api/explain_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,17 @@ pub struct ExplainRequestInput {
pub example: Example,
pub rules: Vec<Rule>,
pub max_hops: u8,
#[serde(default)]
pub project_domains: Vec<String>,
}

#[derive(Deserialize, Debug, Clone)]
pub struct ExplainRequestProjectInput {
pub example: Example,
pub change_set: RuleChangeSet,
pub max_hops: u8,
#[serde(default)]
pub project_domains: Vec<String>,
}

// Output
Expand Down Expand Up @@ -66,6 +70,7 @@ impl ExplainRequestOutput {
&explain_request_router,
&explain_request_input.example,
explain_request_input.max_hops,
explain_request_input.project_domains,
)
}

Expand All @@ -78,10 +83,20 @@ impl ExplainRequestOutput {
router.insert(rule.clone());
}

Self::create_result(&router, &explain_request_input.example, explain_request_input.max_hops)
Self::create_result(
&router,
&explain_request_input.example,
explain_request_input.max_hops,
explain_request_input.project_domains,
)
}

fn create_result(router: &Router<Rule>, example: &Example, max_hops: u8) -> Result<ExplainRequestOutput, ExplainRequestOutputError> {
fn create_result(
router: &Router<Rule>,
example: &Example,
max_hops: u8,
project_domains: Vec<String>,
) -> Result<ExplainRequestOutput, ExplainRequestOutputError> {
let request = match Request::from_example(&router.config, example) {
Ok(request) => request,
Err(e) => {
Expand Down Expand Up @@ -121,7 +136,7 @@ impl ExplainRequestOutput {

unit_trace.squash_with_target_unit_traces();

let redirection_loop = Some(RedirectionLoop::from_example(router, max_hops, example));
let redirection_loop = Some(RedirectionLoop::from_example(router, max_hops, example, project_domains));

Ok(ExplainRequestOutput {
example: example.to_owned(),
Expand Down
10 changes: 9 additions & 1 deletion src/api/impact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ pub struct ImpactInput {
pub router_config: RouterConfig,
pub max_hops: u8,
pub with_redirection_loop: bool,
#[serde(default)]
pub domains: Vec<String>,
pub rule: Rule,
pub action: String,
pub rules: Vec<Rule>,
Expand All @@ -25,6 +27,8 @@ pub struct ImpactInput {
pub struct ImpactProjectInput {
pub max_hops: u8,
pub with_redirection_loop: bool,
#[serde(default)]
pub domains: Vec<String>,
pub rule: Rule,
pub action: String,
pub change_set: RuleChangeSet,
Expand Down Expand Up @@ -88,6 +92,7 @@ impl ImpactOutput {
impact_input.max_hops,
impact_input.action.as_str(),
impact_input.rule,
impact_input.domains,
)
}

Expand All @@ -114,9 +119,11 @@ impl ImpactOutput {
impact_input.max_hops,
impact_input.action.as_str(),
impact_input.rule,
impact_input.domains,
)
}

#[allow(clippy::too_many_arguments)]
fn compute_impacts(
router: &mut Router<Rule>,
trace_unique_router: &mut Router<Rule>,
Expand All @@ -125,6 +132,7 @@ impl ImpactOutput {
max_hops: u8,
action: &str,
rule: Rule,
project_domains: Vec<String>,
) -> ImpactOutput {
if action == "add" || action == "update" {
router.insert(rule.clone());
Expand Down Expand Up @@ -182,7 +190,7 @@ impl ImpactOutput {
unit_trace.squash_with_target_unit_traces();

let redirection_loop = if with_redirection_loop {
Some(RedirectionLoop::from_example(router, max_hops, &example))
Some(RedirectionLoop::from_example(router, max_hops, &example, project_domains.clone()))
} else {
None
};
Expand Down
16 changes: 13 additions & 3 deletions src/api/redirection_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ enum RedirectionError {
}

impl RedirectionLoop {
pub fn from_example(router: &Router<Rule>, max_hops: u8, example: &Example) -> RedirectionLoop {
Self::compute(router, max_hops, example)
pub fn from_example(router: &Router<Rule>, max_hops: u8, example: &Example, project_domains: Vec<String>) -> RedirectionLoop {
Self::compute(router, max_hops, example, project_domains)
}
pub fn has_error(&self) -> bool {
self.error.is_some()
Expand All @@ -40,7 +40,7 @@ impl RedirectionLoop {
pub fn has_error_loop(&self) -> bool {
self.error.is_some() && matches!(self.error, Some(RedirectionError::Loop))
}
fn compute(router: &Router<Rule>, max_hops: u8, example: &Example) -> RedirectionLoop {
fn compute(router: &Router<Rule>, max_hops: u8, example: &Example, project_domains: Vec<String>) -> RedirectionLoop {
let mut current_url = example.url.clone();
let mut current_method = example.method.clone().unwrap_or(String::from("GET"));
let mut error = None;
Expand Down Expand Up @@ -121,6 +121,16 @@ impl RedirectionLoop {
method: current_method.clone(),
});

// If the url cannot be parsed, let's treat it as a relative Url.
// Otherwise, we check if the corresponding domain is registered in the project.
if let Ok(url) = Url::parse(&current_url) {
if !project_domains.is_empty() && !project_domains.contains(&url.host_str().unwrap().to_string()) {
// The current url target a domain that is not registered in the project.
// So we consider there is no redirection loop here.
break;
}
}

if i >= max_hops {
error = Some(RedirectionError::TooManyHops);
break;
Expand Down
27 changes: 22 additions & 5 deletions src/api/test_examples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@ pub struct TestExamplesInput {
pub router_config: RouterConfig,
pub rules: Vec<Rule>,
pub max_hops: u8,
#[serde(default)]
pub project_domains: Vec<String>,
}

#[derive(Deserialize, Debug, Clone, Default)]
pub struct TestExamplesProjectInput {
pub change_set: RuleChangeSet,
pub max_hops: u8,
#[serde(default)]
pub project_domains: Vec<String>,
}

#[derive(Serialize, Debug, Clone, Default)]
Expand Down Expand Up @@ -70,7 +74,11 @@ impl TestExamplesOutput {
pub fn from_project(test_examples_input: TestExamplesProjectInput, existing_router: Arc<Router<Rule>>) -> TestExamplesOutput {
let test_example_router = test_examples_input.change_set.update_existing_router(existing_router);

Self::create_result(&test_example_router, test_examples_input.max_hops)
Self::create_result(
&test_example_router,
test_examples_input.max_hops,
test_examples_input.project_domains,
)
}

pub fn create_result_without_project(test_examples_input: TestExamplesInput) -> TestExamplesOutput {
Expand All @@ -80,10 +88,10 @@ impl TestExamplesOutput {
router.insert(rule.clone());
}

Self::create_result(&router, test_examples_input.max_hops)
Self::create_result(&router, test_examples_input.max_hops, test_examples_input.project_domains)
}

fn create_result(router: &Router<Rule>, max_hops: u8) -> TestExamplesOutput {
fn create_result(router: &Router<Rule>, max_hops: u8, project_domains: Vec<String>) -> TestExamplesOutput {
let mut results = TestExamplesOutput::default();

for (id, route) in router.routes() {
Expand All @@ -94,7 +102,15 @@ impl TestExamplesOutput {
}

for example in examples.as_ref().unwrap().iter() {
Self::test_example(router, example, &mut results, id.as_str(), route.clone(), max_hops);
Self::test_example(
router,
example,
&mut results,
id.as_str(),
route.clone(),
max_hops,
project_domains.clone(),
);
}
}

Expand All @@ -108,6 +124,7 @@ impl TestExamplesOutput {
id: &str,
route: Arc<Route<Rule>>,
max_hops: u8,
project_domains: Vec<String>,
) {
if example.unit_ids_applied.is_none() {
return;
Expand Down Expand Up @@ -172,7 +189,7 @@ impl TestExamplesOutput {
None,
);
} else {
let redirection_loop = RedirectionLoop::from_example(router, max_hops, example);
let redirection_loop = RedirectionLoop::from_example(router, max_hops, example, project_domains);

if redirection_loop.has_error_too_many_hops() || redirection_loop.has_error_loop() {
results.add_failed_example(
Expand Down

0 comments on commit 94b1cec

Please sign in to comment.