Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 21ac6240 authored by Luke Huang's avatar Luke Huang
Browse files

Fix the wrong condition statement in tokio select for doh_handler and correct... 

Fix the wrong condition statement in tokio select for doh_handler and correct some return code in doh_query

1. Currently, the first branch of tokio select in doh_handler is not
strict enough, which causes the first branch can never return PENDING
state if no doh_conn is available. If that happen, tokio select! might
end up comsuming all the budget and get stuck.
To fix this problem, make the precondition stricter.

2. Correct some return code in doh_query, which is used to notify
   DnsResolver fallback to DoT or UDP.

Test: manual test && atest
Bug: 196717322

Change-Id: Ib1f38f2ab7c227780213325a859e19e17fd11d3c
parent a2f85fb6

doh.rs

+17 −6
Original line number Diff line number Diff line
@@ -357,7 +357,6 @@ impl DohConnection { 
                    }

                }

            }

            // TODO: clean up the expired queries.

            self.recv_rx().await?;

            self.flush_tx().await?;

            if let Ok((stream_id, buf)) = self.recv_query() {
@@ -679,6 +678,19 @@ async fn handle_query_cmd( 
        let _ = resp.send(Response::Error { error: QueryError::ServerNotReady });

    }

}

fn need_process_queries(doh_conn_map: &HashMap<u32, (ServerInfo, Option<DohConnection>)>) -> bool {

    if doh_conn_map.is_empty() {

        return false;

    }

    for (_, doh_conn) in doh_conn_map.values() {

        if let Some(doh_conn) = doh_conn {

            if !doh_conn.query_map.is_empty() || !doh_conn.pending_queries.is_empty() {

                return true;

            }

        }

    }

    false

}



async fn doh_handler(

    mut cmd_rx: CmdReceiver,
@@ -703,7 +715,7 @@ async fn doh_handler( 
                    }

                }

                join_all(futures).await

            } , if !doh_conn_map.is_empty() => {},

            }, if need_process_queries(&doh_conn_map) => {},

            Some(result) = probe_futures.next() => {

                let runtime_clone = runtime.clone();

                handle_probe_result(result, &mut doh_conn_map, runtime_clone, validation_fn);
@@ -1015,12 +1027,11 @@ pub unsafe extern "C" fn doh_query( 
                        response.copy_from_slice(&answer);

                        answer.len() as ssize_t

                    }

                    Response::Error { error: QueryError::ServerNotReady } => RESULT_CAN_NOT_SEND,

                    _ => RESULT_INTERNAL_ERROR,

                    _ => RESULT_CAN_NOT_SEND,

                },

                Err(e) => {

                    error!("no result {}", e);

                    RESULT_INTERNAL_ERROR

                    RESULT_CAN_NOT_SEND

                }

            },

            Err(e) => {
@@ -1029,7 +1040,7 @@ pub unsafe extern "C" fn doh_query( 
            }

        }

    } else {

        RESULT_INTERNAL_ERROR

        RESULT_CAN_NOT_SEND

    }

}

tests/resolv_private_dns_test.cpp

+3 −11
Original line number Diff line number Diff line
@@ -285,7 +285,7 @@ TEST_P(TransportParameterizedTest, GetAddrInfo) { 
    if (testParamHasDoh()) {

        EXPECT_NO_FAILURE(expectQueries(0 /* dns */, 0 /* dot */, 2 /* doh */));

    } else {

        EXPECT_NO_FAILURE(expectQueries(2 /* dns */, 0 /* dot */, 0 /* doh */));

        EXPECT_NO_FAILURE(expectQueries(0 /* dns */, 2 /* dot */, 0 /* doh */));

    }



    // Stop the private DNS servers. Since we are in opportunistic mode, queries will
@@ -298,7 +298,7 @@ TEST_P(TransportParameterizedTest, GetAddrInfo) { 
    if (testParamHasDoh()) {

        EXPECT_NO_FAILURE(expectQueries(2 /* dns */, 0 /* dot */, 2 /* doh */));

    } else {

        EXPECT_NO_FAILURE(expectQueries(4 /* dns */, 0 /* dot */, 0 /* doh */));

        EXPECT_NO_FAILURE(expectQueries(2 /* dns */, 2 /* dot */, 0 /* doh */));

    }

}
@@ -357,8 +357,7 @@ TEST_F(PrivateDnsDohTest, ValidationFail) { 


// Tests that DoH query fails and fallback happens.

//   - Fallback to UDP if DoH query times out

//   - Fallback to DoT if DoH validation is in progress.

//   - Fallback to UDP if DoH validation has failed.

//   - Fallback to DoT if DoH validation is in progress or has failed.

TEST_F(PrivateDnsDohTest, QueryFailover) {

    const auto parcel = DnsResponderClient::GetDefaultResolverParamsParcel();

    ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));
@@ -391,13 +390,6 @@ TEST_F(PrivateDnsDohTest, QueryFailover) { 


    EXPECT_EQ(dot.queries(), 2);

    EXPECT_EQ(dns.queries().size(), 0U);

    waitForDohValidationTimeout();

    flushCache();



    // Expect that this query fall back to UDP.

    EXPECT_NO_FAILURE(sendQueryAndCheckResult());

    EXPECT_EQ(dot.queries(), 2);

    EXPECT_EQ(dns.queries().size(), 2U);

}



// Tests that the DnsResolver prioritizes IPv6 DoH servers over IPv4 DoH servers.