Skip to content

Commit

Permalink
der: enable arithmetic and casting clippy lints (#579)
Browse files Browse the repository at this point in the history
Enables the following lints globally:

- `clippy::cast_lossless`
- `clippy::cast_possible_truncation`
- `clippy::cast_possible_wrap`
- `clippy::cast_precision_loss`
- `clippy::cast_sign_loss`
- `clippy::checked_conversions`
- `clippy::implicit_saturating_sub`
- `clippy::integer_arithmetic`

Some code related to datetime-handling and ASN.1 REAL has been
grandfathered in and allows these lints, although it's been labeled with
TODOs to convert it to checked arithmetic.
  • Loading branch information
tarcieri authored Apr 5, 2022
1 parent 71bb668 commit 2633ce6
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 46 deletions.
2 changes: 1 addition & 1 deletion der/src/arrayvec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl<'a, T> Iterator for Iter<'a, T> {
}

fn size_hint(&self) -> (usize, Option<usize>) {
let len = self.elements.len() - self.position;
let len = self.elements.len().saturating_sub(self.position);
(len, Some(len))
}
}
Expand Down
10 changes: 7 additions & 3 deletions der/src/asn1/bit_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ pub struct BitStringIter<'a> {
impl<'a> Iterator for BitStringIter<'a> {
type Item = bool;

#[allow(clippy::integer_arithmetic)]
fn next(&mut self) -> Option<bool> {
if self.position >= self.bit_string.bit_len() {
return None;
Expand All @@ -231,6 +232,7 @@ impl<T: flagset::Flags> FixedTag for flagset::FlagSet<T> {
}

#[cfg(feature = "flagset")]
#[allow(clippy::integer_arithmetic)]
impl<'a, T> DecodeValue<'a> for flagset::FlagSet<T>
where
T: flagset::Flags,
Expand All @@ -256,8 +258,9 @@ where
}

#[cfg(feature = "flagset")]
#[allow(clippy::integer_arithmetic)]
#[inline(always)]
fn encode<T>(set: &flagset::FlagSet<T>) -> (usize, [u8; 16])
fn encode_flagset<T>(set: &flagset::FlagSet<T>) -> (usize, [u8; 16])
where
T: flagset::Flags,
u128: From<T::Type>,
Expand All @@ -274,20 +277,21 @@ where
}

#[cfg(feature = "flagset")]
#[allow(clippy::cast_possible_truncation, clippy::integer_arithmetic)]
impl<T: flagset::Flags> EncodeValue for flagset::FlagSet<T>
where
T::Type: From<bool>,
T::Type: core::ops::Shl<usize, Output = T::Type>,
u128: From<T::Type>,
{
fn value_len(&self) -> Result<Length> {
let (lead, buff) = encode(self);
let (lead, buff) = encode_flagset(self);
let buff = &buff[..buff.len() - lead / 8];
BitString::new((lead % 8) as u8, buff)?.value_len()
}

fn encode_value(&self, encoder: &mut Encoder<'_>) -> Result<()> {
let (lead, buff) = encode(self);
let (lead, buff) = encode_flagset(self);
let buff = &buff[..buff.len() - lead / 8];
BitString::new((lead % 8) as u8, buff)?.encode_value(encoder)
}
Expand Down
16 changes: 10 additions & 6 deletions der/src/asn1/generalized_time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use crate::{
asn1::Any,
datetime::{self, DateTime},
ord::OrdIsValueOrd,
ByteSlice, DecodeValue, Decoder, EncodeValue, Encoder, Error, FixedTag, Header, Length, Result,
Tag,
ByteSlice, DecodeValue, Decoder, EncodeValue, Encoder, Error, ErrorKind, FixedTag, Header,
Length, Result, Tag,
};
use core::time::Duration;

Expand Down Expand Up @@ -78,8 +78,12 @@ impl DecodeValue<'_> for GeneralizedTime {
match *ByteSlice::decode_value(decoder, header)?.as_bytes() {
// RFC 5280 requires mandatory seconds and Z-normalized time zone
[y1, y2, y3, y4, mon1, mon2, day1, day2, hour1, hour2, min1, min2, sec1, sec2, b'Z'] => {
let year = datetime::decode_decimal(Self::TAG, y1, y2)? as u16 * 100
+ datetime::decode_decimal(Self::TAG, y3, y4)? as u16;
let year = u16::from(datetime::decode_decimal(Self::TAG, y1, y2)?)
.checked_mul(100)
.and_then(|y| {
y.checked_add(datetime::decode_decimal(Self::TAG, y3, y4).ok()?.into())
})
.ok_or(ErrorKind::DateTime)?;
let month = datetime::decode_decimal(Self::TAG, mon1, mon2)?;
let day = datetime::decode_decimal(Self::TAG, day1, day2)?;
let hour = datetime::decode_decimal(Self::TAG, hour1, hour2)?;
Expand All @@ -101,8 +105,8 @@ impl EncodeValue for GeneralizedTime {
}

fn encode_value(&self, encoder: &mut Encoder<'_>) -> Result<()> {
let year_hi = (self.0.year() / 100) as u8;
let year_lo = (self.0.year() % 100) as u8;
let year_hi = u8::try_from(self.0.year() / 100)?;
let year_lo = u8::try_from(self.0.year() % 100)?;

datetime::encode_decimal(encoder, Self::TAG, year_hi)?;
datetime::encode_decimal(encoder, Self::TAG, year_lo)?;
Expand Down
2 changes: 1 addition & 1 deletion der/src/asn1/integer/uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub(crate) fn encode_bytes(encoder: &mut Encoder<'_>, bytes: &[u8]) -> Result<()
#[inline]
pub(crate) fn encoded_len(bytes: &[u8]) -> Result<Length> {
let bytes = strip_leading_zeroes(bytes);
Length::try_from(bytes.len())? + needs_leading_zero(bytes) as u8
Length::try_from(bytes.len())? + u8::from(needs_leading_zero(bytes))
}

/// Strip the leading zeroes from the given byte slice
Expand Down
8 changes: 8 additions & 0 deletions der/src/asn1/real.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
//! ASN.1 `REAL` support.
// TODO(tarcieri): checked arithmetic
#![allow(
clippy::cast_lossless,
clippy::cast_sign_loss,
clippy::integer_arithmetic
)]

use crate::{
str_slice::StrSlice, ByteSlice, DecodeValue, Decoder, EncodeValue, Encoder, FixedTag, Header,
Length, Result, Tag,
Expand Down Expand Up @@ -212,6 +219,7 @@ pub(crate) fn mnth_bits_to_u8<const M: usize, const N: usize>(bytes: &[u8]) -> u

/// Decode an f64 as its sign, exponent, and mantissa in u64 and in that order, using bit shifts and masks.
/// Note: this function **removes** the 1023 bias from the exponent and adds the implicit 1
#[allow(clippy::cast_possible_truncation)]
pub(crate) fn decode_f64(f: f64) -> (u64, u64, u64) {
let bits = f.to_bits();
let sign = bits >> 63;
Expand Down
5 changes: 3 additions & 2 deletions der/src/asn1/set_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,10 @@ where
/// This function is used rather than Rust's built-in `[T]::sort_by` in order
/// to support heapless `no_std` targets as well as to enable bubbling up
/// sorting errors.
#[allow(clippy::integer_arithmetic)]
fn der_sort<T: DerOrd>(slice: &mut [T]) -> Result<()> {
for i in 1..=slice.len() {
let mut j = i - 1;
for i in 0..slice.len() {
let mut j = i;

while j > 0 && slice[j - 1].der_cmp(&slice[j])? == Ordering::Greater {
slice.swap(j - 1, j);
Expand Down
21 changes: 12 additions & 9 deletions der/src/asn1/utc_time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use crate::{
asn1::Any,
datetime::{self, DateTime},
ord::OrdIsValueOrd,
ByteSlice, DecodeValue, Decoder, EncodeValue, Encoder, Error, FixedTag, Header, Length, Result,
Tag,
ByteSlice, DecodeValue, Decoder, EncodeValue, Encoder, Error, ErrorKind, FixedTag, Header,
Length, Result, Tag,
};
use core::time::Duration;

Expand Down Expand Up @@ -84,7 +84,7 @@ impl DecodeValue<'_> for UtcTime {
match *ByteSlice::decode_value(decoder, header)?.as_bytes() {
// RFC 5280 requires mandatory seconds and Z-normalized time zone
[year1, year2, mon1, mon2, day1, day2, hour1, hour2, min1, min2, sec1, sec2, b'Z'] => {
let year = datetime::decode_decimal(Self::TAG, year1, year2)?;
let year = u16::from(datetime::decode_decimal(Self::TAG, year1, year2)?);
let month = datetime::decode_decimal(Self::TAG, mon1, mon2)?;
let day = datetime::decode_decimal(Self::TAG, day1, day2)?;
let hour = datetime::decode_decimal(Self::TAG, hour1, hour2)?;
Expand All @@ -93,10 +93,11 @@ impl DecodeValue<'_> for UtcTime {

// RFC 5280 rules for interpreting the year
let year = if year >= 50 {
year as u16 + 1900
year.checked_add(1900)
} else {
year as u16 + 2000
};
year.checked_add(2000)
}
.ok_or(ErrorKind::DateTime)?;

DateTime::new(year, month, day, hour, minute, second)
.map_err(|_| Self::TAG.value_error())
Expand All @@ -114,10 +115,12 @@ impl EncodeValue for UtcTime {

fn encode_value(&self, encoder: &mut Encoder<'_>) -> Result<()> {
let year = match self.0.year() {
y @ 1950..=1999 => y - 1900,
y @ 2000..=2049 => y - 2000,
y @ 1950..=1999 => y.checked_sub(1900),
y @ 2000..=2049 => y.checked_sub(2000),
_ => return Err(Self::TAG.value_error()),
} as u8;
}
.and_then(|y| u8::try_from(y).ok())
.ok_or(ErrorKind::DateTime)?;

datetime::encode_decimal(encoder, Self::TAG, year)?;
datetime::encode_decimal(encoder, Self::TAG, self.0.month())?;
Expand Down
42 changes: 27 additions & 15 deletions der/src/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub struct DateTime {

impl DateTime {
/// Create a new [`DateTime`] from the given UTC time components.
// TODO(tarcieri): checked arithmetic
#[allow(clippy::integer_arithmetic)]
pub fn new(year: u16, month: u8, day: u8, hour: u8, minutes: u8, seconds: u8) -> Result<Self> {
// Basic validation of the components.
if year < MIN_YEAR
Expand Down Expand Up @@ -95,14 +97,14 @@ impl DateTime {
return Err(ErrorKind::DateTime.into());
}

ydays += day as u16 - 1;
ydays += u16::from(day) - 1;

if is_leap_year && month > 2 {
ydays += 1;
}

let days = (year - 1970) as u64 * 365 + leap_years as u64 + ydays as u64;
let time = seconds as u64 + (minutes as u64 * 60) + (hour as u64 * 3600);
let days = u64::from(year - 1970) * 365 + u64::from(leap_years) + u64::from(ydays);
let time = u64::from(seconds) + (u64::from(minutes) * 60) + (u64::from(hour) * 3600);
let unix_duration = Duration::from_secs(time + days * 86400);

if unix_duration > MAX_UNIX_DURATION {
Expand All @@ -123,6 +125,8 @@ impl DateTime {
/// Compute a [`DateTime`] from the given [`Duration`] since the `UNIX_EPOCH`.
///
/// Returns `None` if the value is outside the supported date range.
// TODO(tarcieri): checked arithmetic
#[allow(clippy::integer_arithmetic)]
pub fn from_unix_duration(unix_duration: Duration) -> Result<Self> {
if unix_duration > MAX_UNIX_DURATION {
return Err(ErrorKind::DateTime.into());
Expand All @@ -136,7 +140,7 @@ impl DateTime {
const DAYS_PER_100Y: i64 = 365 * 100 + 24;
const DAYS_PER_4Y: i64 = 365 * 4 + 1;

let days = (secs_since_epoch / 86400) as i64 - LEAPOCH;
let days = i64::try_from(secs_since_epoch / 86400)? - LEAPOCH;
let secs_of_day = secs_since_epoch % 86400;

let mut qc_cycles = days / DAYS_PER_400Y;
Expand Down Expand Up @@ -190,12 +194,12 @@ impl DateTime {
let hour = mins_of_day / 60;

Self::new(
year as u16,
year.try_into()?,
mon,
mday as u8,
hour as u8,
minute as u8,
second as u8,
mday.try_into()?,
hour.try_into()?,
minute.try_into()?,
second.try_into()?,
)
}

Expand Down Expand Up @@ -254,15 +258,19 @@ impl DateTime {
impl FromStr for DateTime {
type Err = Error;

// TODO(tarcieri): checked arithmetic
#[allow(clippy::integer_arithmetic)]
fn from_str(s: &str) -> Result<Self> {
match *s.as_bytes() {
[year1, year2, year3, year4, b'-', month1, month2, b'-', day1, day2, b'T', hour1, hour2, b':', min1, min2, b':', sec1, sec2, b'Z'] =>
{
let tag = Tag::GeneralizedTime;
let year = decode_decimal(tag, year1, year2).map_err(|_| ErrorKind::DateTime)?
as u16
* 100
+ decode_decimal(tag, year3, year4).map_err(|_| ErrorKind::DateTime)? as u16;
let year =
u16::from(decode_decimal(tag, year1, year2).map_err(|_| ErrorKind::DateTime)?)
* 100
+ u16::from(
decode_decimal(tag, year3, year4).map_err(|_| ErrorKind::DateTime)?,
);
let month = decode_decimal(tag, month1, month2).map_err(|_| ErrorKind::DateTime)?;
let day = decode_decimal(tag, day1, day2).map_err(|_| ErrorKind::DateTime)?;
let hour = decode_decimal(tag, hour1, hour2).map_err(|_| ErrorKind::DateTime)?;
Expand Down Expand Up @@ -328,7 +336,7 @@ impl TryFrom<DateTime> for PrimitiveDateTime {

fn try_from(time: DateTime) -> Result<PrimitiveDateTime> {
let month = (time.month() as u8).try_into()?;
let date = time::Date::from_calendar_date(time.year() as i32, month, time.day())?;
let date = time::Date::from_calendar_date(i32::from(time.year()), month, time.day())?;
let time = time::Time::from_hms(time.hour(), time.minutes(), time.seconds())?;

Ok(PrimitiveDateTime::new(date, time))
Expand All @@ -342,7 +350,7 @@ impl TryFrom<PrimitiveDateTime> for DateTime {

fn try_from(time: PrimitiveDateTime) -> Result<DateTime> {
DateTime::new(
time.year() as u16,
time.year().try_into().map_err(|_| ErrorKind::DateTime)?,
time.month().into(),
time.day(),
time.hour(),
Expand All @@ -353,6 +361,8 @@ impl TryFrom<PrimitiveDateTime> for DateTime {
}

/// Decode 2-digit decimal value
// TODO(tarcieri): checked arithmetic
#[allow(clippy::integer_arithmetic)]
pub(crate) fn decode_decimal(tag: Tag, hi: u8, lo: u8) -> Result<u8> {
if (b'0'..=b'9').contains(&hi) && (b'0'..=b'9').contains(&lo) {
Ok((hi - b'0') * 10 + (lo - b'0'))
Expand All @@ -362,6 +372,8 @@ pub(crate) fn decode_decimal(tag: Tag, hi: u8, lo: u8) -> Result<u8> {
}

/// Encode 2-digit decimal value
// TODO(tarcieri): checked arithmetic
#[allow(clippy::integer_arithmetic)]
pub(crate) fn encode_decimal(encoder: &mut Encoder<'_>, tag: Tag, value: u8) -> Result<()> {
let hi_val = value / 10;

Expand Down
11 changes: 10 additions & 1 deletion der/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
pub use core::str::Utf8Error;

use crate::{Length, Tag};
use core::{convert::Infallible, fmt};
use core::{convert::Infallible, fmt, num::TryFromIntError};

#[cfg(feature = "oid")]
use crate::asn1::ObjectIdentifier;
Expand Down Expand Up @@ -91,6 +91,15 @@ impl From<Infallible> for Error {
}
}

impl From<TryFromIntError> for Error {
fn from(_: TryFromIntError) -> Error {
Error {
kind: ErrorKind::Overflow,
position: None,
}
}
}

impl From<Utf8Error> for Error {
fn from(err: Utf8Error) -> Error {
Error {
Expand Down
10 changes: 6 additions & 4 deletions der/src/length.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,13 @@ impl Sub<Length> for Result<Length> {

impl From<u8> for Length {
fn from(len: u8) -> Length {
Length(len as u32)
Length(len.into())
}
}

impl From<u16> for Length {
fn from(len: u16) -> Length {
Length(len as u32)
Length(len.into())
}
}

Expand Down Expand Up @@ -202,9 +202,10 @@ impl Decode<'_> for Length {
let nbytes = tag.checked_sub(0x80).ok_or(ErrorKind::Overlength)? as usize;
debug_assert!(nbytes <= 4);

let mut decoded_len = 0;
let mut decoded_len = 0u32;
for _ in 0..nbytes {
decoded_len = (decoded_len << 8) | decoder.byte()? as u32;
decoded_len = decoded_len.checked_shl(8).ok_or(ErrorKind::Overflow)?
| u32::from(decoder.byte()?);
}

let length = Length::try_from(decoded_len)?;
Expand Down Expand Up @@ -237,6 +238,7 @@ impl Encode for Length {
}
}

#[allow(clippy::cast_possible_truncation)]
fn encode(&self, encoder: &mut Encoder<'_>) -> Result<()> {
if let Some(tag_byte) = self.initial_octet() {
encoder.byte(tag_byte)?;
Expand Down
8 changes: 8 additions & 0 deletions der/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
)]
#![forbid(unsafe_code)]
#![warn(
clippy::cast_lossless,
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::cast_precision_loss,
clippy::cast_sign_loss,
clippy::checked_conversions,
clippy::implicit_saturating_sub,
clippy::integer_arithmetic,
clippy::panic,
clippy::panic_in_result_fn,
clippy::unwrap_used,
Expand Down
Loading

0 comments on commit 2633ce6

Please sign in to comment.