Skip to content

Commit

Permalink
[LLT-4202] Make lookup return error in case of failure on forward
Browse files Browse the repository at this point in the history 
Make the forward error visible instead of just filling the response
buffer with an empty response.
  • Loading branch information
@dfetti
dfetti committed Oct 8, 2023
1 parent 00a31c9 commit d1019f1
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 43 deletions.
89 changes: 46 additions & 43 deletions crates/server/src/authority/catalog.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ impl RequestHandler for Catalog {
MessageType::Query => match request.op_code() {
OpCode::Query => {
debug!("query received: {}", request.id());
let info = self.lookup(request, response_edns, response_handle).await;
let info = self
.lookup(request, response_edns, response_handle)
.await
.unwrap_or_else(|_| ResponseInfo::serve_failed());

Ok(info)
}
Expand Down Expand Up @@ -346,7 +349,7 @@ impl Catalog {
request: &Request,
response_edns: Option<Edns>,
response_handle: R,
) -> ResponseInfo {
) -> Result<ResponseInfo, LookupError> {
let request_info = request.request_info();
let authority = self.find(request_info.query.name());

Expand All @@ -365,20 +368,18 @@ impl Catalog {
// if this is empty then the there are no authorities registered that can handle the request
let response = MessageResponseBuilder::new(Some(request.raw_query()));

let result = send_response(
send_response(
response_edns,
response.error_msg(request.header(), ResponseCode::Refused),
response_handle,
)
.await;
.await
.map_err(|e| {
debug!("failed to send response: {}", e);
LookupError::Io(e)
})?;

match result {
Err(e) => {
debug!("failed to send response: {}", e);
ResponseInfo::serve_failed()
}
Ok(r) => r,
}
Err(LookupError::ResponseCode(ResponseCode::Refused))
}
}

Expand All @@ -405,7 +406,7 @@ async fn lookup<'a, R: ResponseHandler + Unpin>(
request: &Request,
response_edns: Option<Edns>,
response_handle: R,
) -> ResponseInfo {
) -> Result<ResponseInfo, LookupError> {
let query = request_info.query;
debug!(
"request: {} found authority: {}",
Expand All @@ -421,7 +422,7 @@ async fn lookup<'a, R: ResponseHandler + Unpin>(
query,
request.edns(),
)
.await;
.await?;

let response = MessageResponseBuilder::new(Some(request.raw_query())).build(
response_header,
Expand All @@ -431,15 +432,12 @@ async fn lookup<'a, R: ResponseHandler + Unpin>(
sections.additionals.iter(),
);

let result = send_response(response_edns.clone(), response, response_handle.clone()).await;

match result {
Err(e) => {
debug!("error sending response: {}", e);
ResponseInfo::serve_failed()
}
Ok(i) => i,
}
send_response(response_edns.clone(), response, response_handle.clone())
.await
.map_err(|e| {
debug!("failed to send response: {}", e);
LookupError::Io(e)
})
}

#[allow(unused_variables)]
Expand Down Expand Up @@ -473,7 +471,7 @@ async fn build_response(
request_header: &Header,
query: &LowerQuery,
edns: Option<&Edns>,
) -> (Header, LookupSections) {
) -> Result<(Header, LookupSections), LookupError> {
let lookup_options = lookup_options_for_edns(edns);

// log algorithms being requested
Expand Down Expand Up @@ -501,14 +499,14 @@ async fn build_response(
request_id,
query,
)
.await
.await?
}
ZoneType::Forward | ZoneType::Hint => {
send_forwarded_response(future, request_header, &mut response_header).await
send_forwarded_response(future, request_header, &mut response_header).await?
}
};

(response_header, sections)
Ok((response_header, sections))
}

async fn send_authoritative_response(
Expand All @@ -518,7 +516,7 @@ async fn send_authoritative_response(
lookup_options: LookupOptions,
request_id: u16,
query: &LowerQuery,
) -> LookupSections {
) -> Result<LookupSections, LookupError> {
// In this state we await the records, on success we transition to getting
// NS records, which indicate an authoritative response.
//
Expand All @@ -533,12 +531,12 @@ async fn send_authoritative_response(
// TODO: there are probably other error cases that should just drop through (FormErr, ServFail)
Err(LookupError::ResponseCode(ResponseCode::Refused)) => {
response_header.set_response_code(ResponseCode::Refused);
return LookupSections {
answers: Box::new(AuthLookup::default()) as Box<dyn LookupObject>,
ns: Box::new(AuthLookup::default()) as Box<dyn LookupObject>,
soa: Box::new(AuthLookup::default()) as Box<dyn LookupObject>,
additionals: Box::new(AuthLookup::default()) as Box<dyn LookupObject>,
};
return Ok(LookupSections {
answers: Box::<AuthLookup>::default(),
ns: Box::<AuthLookup>::default(),
soa: Box::<AuthLookup>::default(),
additionals: Box::<AuthLookup>::default(),
});
}
Err(e) => {
if e.is_nx_domain() {
Expand Down Expand Up @@ -607,19 +605,19 @@ async fn send_authoritative_response(
),
};

LookupSections {
Ok(LookupSections {
answers,
ns: ns.unwrap_or_else(|| Box::new(AuthLookup::default()) as Box<dyn LookupObject>),
soa: soa.unwrap_or_else(|| Box::new(AuthLookup::default()) as Box<dyn LookupObject>),
additionals,
}
})
}

async fn send_forwarded_response(
future: impl Future<Output = Result<Box<dyn LookupObject>, LookupError>>,
request_header: &Header,
response_header: &mut Header,
) -> LookupSections {
) -> Result<LookupSections, LookupError> {
response_header.set_recursion_available(true);
response_header.set_authoritative(false);

Expand All @@ -634,26 +632,31 @@ async fn send_forwarded_response(
request_header.id()
);

Box::new(EmptyLookup)
return Err(LookupError::ResponseCode(ResponseCode::Refused));
} else {
match future.await {
Err(e) => {
debug!("error resolving: {}", e);
if e.is_io() || e.is_unknown() {
return Err(e);
}

// If the server responds with NXDomain we want to copy this behavior
if e.is_nx_domain() {
response_header.set_response_code(ResponseCode::NXDomain);
}
debug!("error resolving: {}", e);
Box::new(EmptyLookup)
}
Ok(rsp) => rsp,
}
};

LookupSections {
Ok(LookupSections {
answers,
ns: Box::new(AuthLookup::default()) as Box<dyn LookupObject>,
soa: Box::new(AuthLookup::default()) as Box<dyn LookupObject>,
additionals: Box::new(AuthLookup::default()) as Box<dyn LookupObject>,
}
ns: Box::<AuthLookup>::default(),
soa: Box::<AuthLookup>::default(),
additionals: Box::<AuthLookup>::default(),
})
}

struct LookupSections {
Expand Down
10 changes: 10 additions & 0 deletions crates/server/src/authority/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ impl LookupError {
pub fn is_refused(&self) -> bool {
matches!(*self, Self::ResponseCode(ResponseCode::Refused))
}

/// This is an Io error
pub fn is_io(&self) -> bool {
matches!(*self, Self::Io(_))
}

/// This is an unknown error
pub fn is_unknown(&self) -> bool {
matches!(*self, Self::ResponseCode(ResponseCode::Unknown(_)))
}
}

impl From<ResponseCode> for LookupError {
Expand Down

0 comments on commit d1019f1

Please sign in to comment.