Skip to content

Commit c9f1896

Browse files
committed
Less unwrap
1 parent af99cbc commit c9f1896

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+571
-426
lines changed

.clippy.toml

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
disallowed-methods = [
22
{ path = "std::slice::from_raw_parts", reason = "see null_safe_slice" }
33
]
4+
allow-unwrap-in-tests = true

Cargo.toml

+2-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ url = { version = "2.5.3", default-features = false, features = ["std"] }
4242
cargo = { level = "warn", priority = -1 }
4343
nursery = { level = "warn", priority = -1 }
4444
pedantic = { level = "warn", priority = -1 }
45-
multiple_crate_versions = "allow"
45+
unwrap_used = "warn"
46+
unwrap_in_result = "warn"
4647

4748
# Optimize build dependencies, because bindgen and proc macros / style
4849
# compilation take more to run than to build otherwise.

neqo-bin/src/client/http09.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// option. This file may not be copied, modified, or distributed
55
// except according to those terms.
66

7+
#![allow(clippy::unwrap_used)] // This is example code.
8+
79
//! An [HTTP 0.9](https://www.w3.org/Protocols/HTTP/AsImplemented.html) client implementation.
810
911
use std::{
@@ -159,7 +161,11 @@ pub fn create_client(
159161
client.set_ciphers(&ciphers)?;
160162
}
161163

162-
client.set_qlog(qlog_new(args, hostname, client.odcid().unwrap())?);
164+
client.set_qlog(qlog_new(
165+
args,
166+
hostname,
167+
client.odcid().ok_or(Error::InternalError)?,
168+
)?);
163169

164170
Ok(client)
165171
}
@@ -303,7 +309,7 @@ impl<'b> Handler<'b> {
303309
qdebug!(
304310
"READ[{}]: {}",
305311
stream_id,
306-
std::str::from_utf8(read_buffer).unwrap()
312+
std::str::from_utf8(read_buffer).map_err(|_| Error::InvalidInput)?
307313
);
308314
}
309315
if fin {

neqo-bin/src/client/http3.rs

+17-12
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// option. This file may not be copied, modified, or distributed
55
// except according to those terms.
66

7+
#![allow(clippy::unwrap_used)] // This is example code.
8+
79
//! An HTTP 3 client implementation.
810
911
use std::{
@@ -18,7 +20,7 @@ use std::{
1820
time::Instant,
1921
};
2022

21-
use neqo_common::{event::Provider, hex, qdebug, qinfo, qwarn, Datagram, Header};
23+
use neqo_common::{event::Provider, hex, qdebug, qerror, qinfo, qwarn, Datagram, Header};
2224
use neqo_crypto::{AuthenticationStatus, ResumptionToken};
2325
use neqo_http3::{Error, Http3Client, Http3ClientEvent, Http3Parameters, Http3State, Priority};
2426
use neqo_transport::{
@@ -200,9 +202,11 @@ impl super::Handler for Handler<'_> {
200202
qwarn!("Data on unexpected stream: {stream_id}");
201203
}
202204
Some(handler) => loop {
203-
let (sz, fin) = client
204-
.read_data(Instant::now(), stream_id, &mut self.read_buffer)
205-
.expect("Read should succeed");
205+
let (sz, fin) = client.read_data(
206+
Instant::now(),
207+
stream_id,
208+
&mut self.read_buffer,
209+
)?;
206210

207211
handler.process_data_readable(
208212
stream_id,
@@ -270,7 +274,7 @@ trait StreamHandler {
270274
fin: bool,
271275
data: &[u8],
272276
output_read_data: bool,
273-
) -> Res<bool>;
277+
) -> Res<()>;
274278
fn process_data_writable(&mut self, client: &mut Http3Client, stream_id: StreamId);
275279
}
276280

@@ -291,12 +295,12 @@ impl StreamHandler for DownloadStreamHandler {
291295
fin: bool,
292296
data: &[u8],
293297
output_read_data: bool,
294-
) -> Res<bool> {
298+
) -> Res<()> {
295299
if let Some(out_file) = &mut self.out_file {
296300
if !data.is_empty() {
297301
out_file.write_all(data)?;
298302
}
299-
return Ok(true);
303+
return Ok(());
300304
} else if !output_read_data {
301305
qdebug!("READ[{stream_id}]: {} bytes", data.len());
302306
} else if let Ok(txt) = std::str::from_utf8(data) {
@@ -313,7 +317,7 @@ impl StreamHandler for DownloadStreamHandler {
313317
}
314318
}
315319

316-
Ok(true)
320+
Ok(())
317321
}
318322

319323
fn process_data_writable(&mut self, _client: &mut Http3Client, _stream_id: StreamId) {}
@@ -335,18 +339,19 @@ impl StreamHandler for UploadStreamHandler {
335339
_fin: bool,
336340
data: &[u8],
337341
_output_read_data: bool,
338-
) -> Res<bool> {
342+
) -> Res<()> {
339343
if let Ok(txt) = std::str::from_utf8(data) {
340344
let trimmed_txt = txt.trim_end_matches(char::from(0));
341-
let parsed: usize = trimmed_txt.parse().unwrap();
345+
let parsed: usize = trimmed_txt.parse().map_err(|_| Error::InvalidInput)?;
342346
if parsed == self.data.len() {
343347
let upload_time = Instant::now().duration_since(self.start);
344348
qinfo!("Stream ID: {stream_id:?}, Upload time: {upload_time:?}");
345349
}
350+
Ok(())
346351
} else {
347-
panic!("Unexpected data [{}]: 0x{}", stream_id, hex(data));
352+
qerror!("Unexpected data [{}]: 0x{}", stream_id, hex(data));
353+
Err(crate::client::Error::Http3Error(Error::InvalidInput))
348354
}
349-
Ok(true)
350355
}
351356

352357
fn process_data_writable(&mut self, client: &mut Http3Client, stream_id: StreamId) {

neqo-bin/src/client/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// except according to those terms.
66

77
#![allow(clippy::future_not_send)]
8+
#![allow(clippy::unwrap_used)] // This is example code.
89

910
use std::{
1011
collections::{HashMap, VecDeque},

neqo-bin/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,8 @@ impl QuicParameters {
163163
assert_eq!(
164164
opt.is_some(),
165165
addr.is_some(),
166-
"unable to resolve '{}' to an {} address",
167-
opt.as_ref().unwrap(),
166+
"unable to resolve '{:?}' to an {} address",
167+
opt.as_ref(),
168168
v,
169169
);
170170
addr

neqo-bin/src/server/http09.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// option. This file may not be copied, modified, or distributed
55
// except according to those terms.
66

7+
#![allow(clippy::unwrap_used)] // This is example code.
8+
79
use std::{borrow::Cow, cell::RefCell, collections::HashMap, fmt::Display, rc::Rc, time::Instant};
810

911
use neqo_common::{event::Provider, hex, qdebug, qerror, qinfo, qwarn, Datagram};
@@ -55,10 +57,10 @@ impl HttpServer {
5557
server.set_validation(ValidateAddress::Always);
5658
}
5759
if args.ech {
58-
let (sk, pk) = generate_ech_keys().expect("generate ECH keys");
60+
let (sk, pk) = generate_ech_keys().map_err(|_| Error::Internal)?;
5961
server
6062
.enable_ech(random::<1>()[0], "public.example", &sk, &pk)
61-
.expect("enable ECH");
63+
.map_err(|_| Error::Internal)?;
6264
let cfg = server.ech_config();
6365
qinfo!("ECHConfigList: {}", hex(cfg));
6466
}
@@ -70,9 +72,9 @@ impl HttpServer {
7072
read_state: HashMap::new(),
7173
is_qns_test,
7274
regex: if is_qns_test {
73-
Regex::new(r"GET +/(\S+)(?:\r)?\n").unwrap()
75+
Regex::new(r"GET +/(\S+)(?:\r)?\n").map_err(|_| Error::Internal)?
7476
} else {
75-
Regex::new(r"GET +/(\d+)(?:\r)?\n").unwrap()
77+
Regex::new(r"GET +/(\d+)(?:\r)?\n").map_err(|_| Error::Internal)?
7678
},
7779
read_buffer: vec![0; STREAM_IO_BUFFER_SIZE],
7880
})

neqo-bin/src/server/http3.rs

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// option. This file may not be copied, modified, or distributed
55
// except according to those terms.
66

7+
#![allow(clippy::unwrap_used)] // This is example code.
8+
79
use std::{
810
cell::RefCell,
911
collections::HashMap,

neqo-bin/src/server/mod.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// except according to those terms.
66

77
#![allow(clippy::future_not_send)]
8+
#![allow(clippy::unwrap_used)] // This is example code.
89

910
use std::{
1011
cell::RefCell,
@@ -263,7 +264,10 @@ impl ServerRunner {
263264

264265
async fn read_and_process(&mut self, sockets_index: usize) -> Result<(), io::Error> {
265266
loop {
266-
let (host, socket) = self.sockets.get_mut(sockets_index).unwrap();
267+
let (host, socket) = self
268+
.sockets
269+
.get_mut(sockets_index)
270+
.ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "No socket"))?;
267271
let Some(input_dgrams) = socket.recv(*host, &mut self.recv_buf)? else {
268272
break;
269273
};

neqo-common/build.rs

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// option. This file may not be copied, modified, or distributed
55
// except according to those terms.
66

7+
#![allow(clippy::unwrap_used)] // OK in a build script.
8+
79
use std::env;
810

911
fn main() {

neqo-common/src/codec.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ impl<'a> Decoder<'a> {
4747
/// Only use this for tests because we panic rather than reporting a result.
4848
#[cfg(any(test, feature = "test-fixture"))]
4949
fn skip_inner(&mut self, n: Option<u64>) {
50+
#[allow(clippy::unwrap_used)] // Only used in tests.
5051
self.skip(usize::try_from(n.expect("invalid length")).unwrap());
5152
}
5253

@@ -227,7 +228,7 @@ impl Encoder {
227228
/// When `len` doesn't fit in a `u64`.
228229
#[must_use]
229230
pub fn vvec_len(len: usize) -> usize {
230-
Self::varint_len(u64::try_from(len).unwrap()) + len
231+
Self::varint_len(u64::try_from(len).expect("usize should fit into u64")) + len
231232
}
232233

233234
/// Default construction of an empty buffer.
@@ -286,6 +287,7 @@ impl Encoder {
286287
let mut enc = Self::with_capacity(cap);
287288

288289
for i in 0..cap {
290+
#[allow(clippy::unwrap_used)] // Only used in tests.
289291
let v = u8::from_str_radix(&s[i * 2..i * 2 + 2], 16).unwrap();
290292
enc.encode_byte(v);
291293
}
@@ -341,8 +343,11 @@ impl Encoder {
341343
///
342344
/// When `v` is longer than 2^64.
343345
pub fn encode_vec(&mut self, n: usize, v: &[u8]) -> &mut Self {
344-
self.encode_uint(n, u64::try_from(v.as_ref().len()).unwrap())
345-
.encode(v)
346+
self.encode_uint(
347+
n,
348+
u64::try_from(v.as_ref().len()).expect("v is longer than 2^64"),
349+
)
350+
.encode(v)
346351
}
347352

348353
/// Encode a vector in TLS style using a closure for the contents.
@@ -369,7 +374,7 @@ impl Encoder {
369374
///
370375
/// When `v` is longer than 2^64.
371376
pub fn encode_vvec(&mut self, v: &[u8]) -> &mut Self {
372-
self.encode_varint(u64::try_from(v.as_ref().len()).unwrap())
377+
self.encode_varint(u64::try_from(v.as_ref().len()).expect("v is longer than 2^64"))
373378
.encode(v)
374379
}
375380

neqo-common/src/hrtime.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ impl PeriodSet {
7070
fn min(&self) -> Option<Period> {
7171
for (i, v) in self.counts.iter().enumerate() {
7272
if *v > 0 {
73-
return Some(Period(u8::try_from(i).unwrap() + Period::MIN.0));
73+
return Some(Period(u8::try_from(i).ok()? + Period::MIN.0));
7474
}
7575
}
7676
None
@@ -412,6 +412,7 @@ mod test {
412412

413413
/// Validate the delays twice. Sometimes the first run can stall.
414414
/// Reliability in CI is more important than reliable timers.
415+
#[cfg(test)]
415416
fn check_delays(max_lag: Duration) {
416417
if validate_delays(max_lag).is_err() {
417418
sleep(Duration::from_millis(50));

neqo-common/src/incrdecoder.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ impl IncrementalDecoderUint {
3131
if amount < 8 {
3232
self.v <<= amount * 8;
3333
}
34-
self.v |= dv.decode_n(amount).unwrap();
34+
self.v |= dv.decode_n(amount)?;
3535
*r -= amount;
3636
if *r == 0 {
3737
Some(self.v)
@@ -97,7 +97,7 @@ impl IncrementalDecoderBuffer {
9797
/// Never; but rust doesn't know that.
9898
pub fn consume(&mut self, dv: &mut Decoder) -> Option<Vec<u8>> {
9999
let amount = min(self.remaining, dv.remaining());
100-
let b = dv.decode(amount).unwrap();
100+
let b = dv.decode(amount)?;
101101
self.v.extend_from_slice(b);
102102
self.remaining -= amount;
103103
if self.remaining == 0 {

neqo-crypto/build.rs

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// option. This file may not be copied, modified, or distributed
55
// except according to those terms.
66

7+
#![allow(clippy::unwrap_used)] // OK in a build script.
8+
79
use std::{
810
collections::HashMap,
911
env, fs,

neqo-crypto/src/agent.rs

+17-16
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// option. This file may not be copied, modified, or distributed
55
// except according to those terms.
66

7+
#![allow(clippy::unwrap_used)] // Let's assume the use of `unwrap` was checked when the use of `unsafe` was reviewed.
8+
79
use std::{
810
cell::RefCell,
911
ffi::{CStr, CString},
@@ -122,7 +124,7 @@ macro_rules! preinfo_arg {
122124
pub fn $v(&self) -> Option<$t> {
123125
match self.info.valuesSet & ssl::$m {
124126
0 => None,
125-
_ => Some(<$t>::try_from(self.info.$f).unwrap()),
127+
_ => Some(<$t>::try_from(self.info.$f).ok()?),
126128
}
127129
}
128130
};
@@ -158,12 +160,11 @@ impl SecretAgentPreInfo {
158160
self.info.canSendEarlyData != 0
159161
}
160162

161-
/// # Panics
163+
/// # Errors
162164
///
163165
/// If `usize` is less than 32 bits and the value is too large.
164-
#[must_use]
165-
pub fn max_early_data(&self) -> usize {
166-
usize::try_from(self.info.maxEarlyDataSize).unwrap()
166+
pub fn max_early_data(&self) -> Res<usize> {
167+
usize::try_from(self.info.maxEarlyDataSize).map_err(|_| Error::InternalError)
167168
}
168169

169170
/// Was ECH accepted.
@@ -542,9 +543,7 @@ impl SecretAgent {
542543

543544
// NSS inherited an idiosyncratic API as a result of having implemented NPN
544545
// before ALPN. For that reason, we need to put the "best" option last.
545-
let (first, rest) = protocols
546-
.split_first()
547-
.expect("at least one ALPN value needed");
546+
let (first, rest) = protocols.split_first().ok_or(Error::InternalError)?;
548547
for v in rest {
549548
add(v.as_ref());
550549
}
@@ -872,12 +871,10 @@ impl Client {
872871
arg: *mut c_void,
873872
) -> ssl::SECStatus {
874873
let mut info: MaybeUninit<ssl::SSLResumptionTokenInfo> = MaybeUninit::uninit();
875-
let info_res = &ssl::SSL_GetResumptionTokenInfo(
876-
token,
877-
len,
878-
info.as_mut_ptr(),
879-
c_uint::try_from(mem::size_of::<ssl::SSLResumptionTokenInfo>()).unwrap(),
880-
);
874+
let Ok(info_len) = c_uint::try_from(mem::size_of::<ssl::SSLResumptionTokenInfo>()) else {
875+
return ssl::SECFailure;
876+
};
877+
let info_res = &ssl::SSL_GetResumptionTokenInfo(token, len, info.as_mut_ptr(), info_len);
881878
if info_res.is_err() {
882879
// Ignore the token.
883880
return ssl::SECSuccess;
@@ -887,8 +884,12 @@ impl Client {
887884
// Ignore the token.
888885
return ssl::SECSuccess;
889886
}
890-
let resumption = arg.cast::<Vec<ResumptionToken>>().as_mut().unwrap();
891-
let len = usize::try_from(len).unwrap();
887+
let Some(resumption) = arg.cast::<Vec<ResumptionToken>>().as_mut() else {
888+
return ssl::SECFailure;
889+
};
890+
let Ok(len) = usize::try_from(len) else {
891+
return ssl::SECFailure;
892+
};
892893
let mut v = Vec::with_capacity(len);
893894
v.extend_from_slice(null_safe_slice(token, len));
894895
qdebug!(

0 commit comments

Comments
 (0)