Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion etherparse/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ rust-version = "1.83.0"

[features]
default = ["std"]
std = ["arrayvec/std"]
alloc = []
std = ["alloc", "arrayvec/std"]

[dependencies]
arrayvec = { version = "0.7.2", default-features = false }
Expand Down
109 changes: 109 additions & 0 deletions etherparse/src/err/packet/build_slice_write_error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
use crate::err::{ipv4_exts, ipv6_exts, SliceWriteSpaceError, ValueTooBigError};

/// Error while serializing a packet into a byte slice.
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub enum BuildSliceWriteError {
/// Not enough space is available in the target slice.
/// Contains the minimum required length.
Space(usize),

/// Error if the length of the payload is too
/// big to be representable by the length fields.
PayloadLen(ValueTooBigError<usize>),

/// Error if the IPv4 extensions can not be serialized
/// because of internal consistency errors.
Ipv4Exts(ipv4_exts::ExtsWalkError),
Comment on lines +14 to +16
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clean up the new public error text.

The IPv4 extension docs end with an incomplete sentence, and the ICMPv6/IPv4 display message has a grammar issue.

📝 Proposed wording cleanup
-    /// because of internal consistency errors (i.e. a header
-    /// is never).
+    /// because of internal consistency errors.
...
-                "Error: ICMPv6 can not be combined with an IPv4 headers (checksum can not be calculated)."
+                "Error: ICMPv6 can not be combined with an IPv4 header (checksum can not be calculated)."

Also applies to: 90-92

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@etherparse/src/err/packet/build_slice_write_error.rs` around lines 14 - 17,
The doc comment for the enum variant Ipv4Exts is incomplete and the human-facing
message for the ICMPv6/IPv4 case has a grammar issue; update the Ipv4Exts doc to
a complete sentence (e.g., "Error when IPv4 extensions cannot be serialized due
to internal consistency errors (e.g., an inconsistent header).") and fix the
Display text in the relevant fmt implementation for BuildSliceWriteError (or the
Display impl that formats the ICMPv6/IPv4 case) to use correct grammar/wording
such as "failed to serialize ICMPv6/IPv4 message" or "could not serialize
ICMPv6/IPv4 message" and ensure "can not" is changed to "cannot" where
applicable.


/// Error if the IPv6 extensions can not be serialized
/// because of internal consistency errors.
Ipv6Exts(ipv6_exts::ExtsWalkError),

/// Error if ICMPv6 is packaged in an IPv4 packet (it is undefined
/// how to calculate the checksum).
Icmpv6InIpv4,

/// Address size defined in the ARP header does not match the actual size.
ArpHeaderNotMatch,
}

impl From<ValueTooBigError<usize>> for BuildSliceWriteError {
fn from(value: ValueTooBigError<usize>) -> Self {
BuildSliceWriteError::PayloadLen(value)
}
}

impl From<SliceWriteSpaceError> for BuildSliceWriteError {
fn from(value: SliceWriteSpaceError) -> Self {
BuildSliceWriteError::Space(value.required_len)
}
}

impl From<super::TransportChecksumError> for BuildSliceWriteError {
fn from(value: super::TransportChecksumError) -> Self {
match value {
super::TransportChecksumError::PayloadLen(err) => BuildSliceWriteError::PayloadLen(err),
super::TransportChecksumError::Icmpv6InIpv4 => BuildSliceWriteError::Icmpv6InIpv4,
}
}
}

impl From<crate::WriteError<SliceWriteSpaceError, ipv4_exts::ExtsWalkError>>
for BuildSliceWriteError
{
fn from(value: crate::WriteError<SliceWriteSpaceError, ipv4_exts::ExtsWalkError>) -> Self {
match value {
crate::WriteError::Io(err) => BuildSliceWriteError::Space(err.required_len),
crate::WriteError::Content(err) => BuildSliceWriteError::Ipv4Exts(err),
}
}
}

impl From<crate::WriteError<SliceWriteSpaceError, ipv6_exts::ExtsWalkError>>
for BuildSliceWriteError
{
fn from(value: crate::WriteError<SliceWriteSpaceError, ipv6_exts::ExtsWalkError>) -> Self {
match value {
crate::WriteError::Io(err) => BuildSliceWriteError::Space(err.required_len),
crate::WriteError::Content(err) => BuildSliceWriteError::Ipv6Exts(err),
}
}
}
Comment on lines +36 to +71
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find any code that constructs/returns BuildSliceWriteError from a SliceWriteSpaceError
# or from WriteError<SliceWriteSpaceError, _>.
rg -nP -C3 'SliceWriteSpaceError' --type=rust
echo '--- call sites returning BuildSliceWriteError ---'
rg -nP -C2 'BuildSliceWriteError' --type=rust
echo '--- SliceCoreWriteError usage ---'
rg -nP -C2 'SliceCoreWriteError' --type=rust

Repository: JulianSchmid/etherparse

Length of output: 25197


🏁 Script executed:

# Search for places where SliceWriteSpaceError is converted to BuildSliceWriteError
# Look for .into() calls, ? operators, or explicit From::from calls
rg -n 'SliceWriteSpaceError' --type=rust -A 5 | grep -E '(\.into\(\)|\.from\(|BuildSliceWriteError)' -B 2 -A 1

# Also check if there are tests specifically for these From impls
fd -e rs --type f | xargs grep -l 'BuildSliceWriteError' | xargs grep -l 'SliceWriteSpaceError'

# Search for uses of the individual header write_to_slice methods
rg 'write_to_slice' --type=rust -B 2 -A 3

Repository: JulianSchmid/etherparse

Length of output: 11984


🏁 Script executed:

# Check visibility and how BuildSliceWriteError is exported
rg -n 'pub.*BuildSliceWriteError' --type=rust

# Check the error enum structure and variant documentation
cat -n etherparse/src/err/packet/build_slice_write_error.rs | head -35

# Check if there's any documentation suggesting these conversions should be used
rg -B 3 'From<SliceWriteSpaceError>' etherparse/src/err/packet/build_slice_write_error.rs

# Look for any public API that might return these error types mixed
rg 'pub.*fn.*BuildSliceWriteError' --type=rust -A 2

Repository: JulianSchmid/etherparse

Length of output: 2029


Remove unused From<SliceWriteSpaceError> impls or add code paths that use them.

The three From impls for SliceWriteSpaceError in this file (lines 36–40 and 51–71) have no reachable call sites. The active packet builder pipeline uses SliceCoreWriteError exclusively, and individual header write_to_slice() methods (e.g., ethernet2_header, linux_sll_header) return SliceWriteSpaceError directly rather than through BuildSliceWriteError. If these conversions are not needed, drop them; otherwise, add explicit code paths and tests that exercise them.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@etherparse/src/err/packet/build_slice_write_error.rs` around lines 36 - 71,
The file contains unused From impls that convert SliceWriteSpaceError (the
direct impl From<SliceWriteSpaceError> for BuildSliceWriteError and the two
impls From<crate::WriteError<SliceWriteSpaceError, ipv4_exts::ExtsWalkError>>
and From<crate::WriteError<SliceWriteSpaceError, ipv6_exts::ExtsWalkError>>),
which the current pipeline never invokes; either remove these three impl blocks
(the From<SliceWriteSpaceError> impl and the two
WriteError->BuildSliceWriteError impls) to avoid dead code, or add explicit code
paths and tests that exercise them (e.g., adapt a builder or header write path
to return/propagate BuildSliceWriteError via these conversions and add unit
tests asserting the conversion produces the expected BuildSliceWriteError
variants). Ensure you update or add tests to cover the chosen approach and
remove any now-unnecessary imports.


impl core::fmt::Display for BuildSliceWriteError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
use BuildSliceWriteError::*;
match self {
Space(required_len) => write!(
f,
"Not enough space to write packet to slice. Needed {} byte(s).",
required_len
),
PayloadLen(err) => err.fmt(f),
Ipv4Exts(err) => err.fmt(f),
Ipv6Exts(err) => err.fmt(f),
ArpHeaderNotMatch => write!(
f,
"address size defined in the ARP header does not match the actual size"
),
Icmpv6InIpv4 => write!(
f,
"Error: ICMPv6 can not be combined with an IPv4 header (checksum can not be calculated)."
),
}
}
}

impl core::error::Error for BuildSliceWriteError {
fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
use BuildSliceWriteError::*;
match self {
Space(_) => None,
PayloadLen(err) => Some(err),
Ipv4Exts(err) => Some(err),
Ipv6Exts(err) => Some(err),
Icmpv6InIpv4 => None,
ArpHeaderNotMatch => None,
}
}
}
97 changes: 97 additions & 0 deletions etherparse/src/err/packet/build_vec_write_error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
use crate::err::{ipv4_exts, ipv6_exts, ValueTooBigError};
use core::convert::Infallible;

/// Error while serializing a packet into a [`alloc::vec::Vec`].
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub enum BuildVecWriteError {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
JulianSchmid marked this conversation as resolved.
/// Error if the length of the payload is too
/// big to be representable by the length fields.
PayloadLen(ValueTooBigError<usize>),

/// Error if the IPv4 extensions can not be serialized
/// because of internal consistency errors.
Ipv4Exts(ipv4_exts::ExtsWalkError),

/// Error if the IPv6 extensions can not be serialized
/// because of internal consistency errors.
Ipv6Exts(ipv6_exts::ExtsWalkError),

/// Error if ICMPv6 is packaged in an IPv4 packet (it is undefined
/// how to calculate the checksum).
Icmpv6InIpv4,

/// Address size defined in the ARP header does not match the actual size.
ArpHeaderNotMatch,
}

impl From<Infallible> for BuildVecWriteError {
fn from(value: Infallible) -> Self {
match value {}
}
}

impl From<ValueTooBigError<usize>> for BuildVecWriteError {
fn from(value: ValueTooBigError<usize>) -> Self {
BuildVecWriteError::PayloadLen(value)
}
}

impl From<super::TransportChecksumError> for BuildVecWriteError {
fn from(value: super::TransportChecksumError) -> Self {
match value {
super::TransportChecksumError::PayloadLen(err) => BuildVecWriteError::PayloadLen(err),
super::TransportChecksumError::Icmpv6InIpv4 => BuildVecWriteError::Icmpv6InIpv4,
}
}
}

impl From<crate::WriteError<Infallible, ipv4_exts::ExtsWalkError>> for BuildVecWriteError {
fn from(value: crate::WriteError<Infallible, ipv4_exts::ExtsWalkError>) -> Self {
match value {
crate::WriteError::Io(err) => match err {},
crate::WriteError::Content(err) => BuildVecWriteError::Ipv4Exts(err),
}
}
}

impl From<crate::WriteError<Infallible, ipv6_exts::ExtsWalkError>> for BuildVecWriteError {
fn from(value: crate::WriteError<Infallible, ipv6_exts::ExtsWalkError>) -> Self {
match value {
crate::WriteError::Io(err) => match err {},
crate::WriteError::Content(err) => BuildVecWriteError::Ipv6Exts(err),
}
}
}

impl core::fmt::Display for BuildVecWriteError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
use BuildVecWriteError::*;
match self {
PayloadLen(err) => err.fmt(f),
Ipv4Exts(err) => err.fmt(f),
Ipv6Exts(err) => err.fmt(f),
ArpHeaderNotMatch => write!(
f,
"address size defined in the ARP header does not match the actual size"
),
Icmpv6InIpv4 => write!(
f,
"Error: ICMPv6 can not be combined with an IPv4 headers (checksum can not be calculated)."
),
}
}
}

impl core::error::Error for BuildVecWriteError {
fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
use BuildVecWriteError::*;
match self {
PayloadLen(err) => Some(err),
Ipv4Exts(err) => Some(err),
Ipv6Exts(err) => Some(err),
Icmpv6InIpv4 => None,
ArpHeaderNotMatch => None,
}
}
}
47 changes: 45 additions & 2 deletions etherparse/src/err/packet/build_write_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ pub enum BuildWriteError {
PayloadLen(ValueTooBigError<usize>),

/// Error if the IPv4 extensions can not be serialized
/// because of internal consistency errors (i.e. a header
/// is never).
/// because of internal consistency errors.
Ipv4Exts(ipv4_exts::ExtsWalkError),

/// Error if the IPv6 extensions can not be serialized
Expand Down Expand Up @@ -105,6 +104,50 @@ impl core::error::Error for BuildWriteError {
}
}

#[cfg(feature = "std")]
impl From<std::io::Error> for BuildWriteError {
fn from(value: std::io::Error) -> Self {
BuildWriteError::Io(value)
}
}

#[cfg(feature = "std")]
impl From<ValueTooBigError<usize>> for BuildWriteError {
fn from(value: ValueTooBigError<usize>) -> Self {
BuildWriteError::PayloadLen(value)
}
}

#[cfg(feature = "std")]
impl From<super::TransportChecksumError> for BuildWriteError {
fn from(value: super::TransportChecksumError) -> Self {
match value {
super::TransportChecksumError::PayloadLen(err) => BuildWriteError::PayloadLen(err),
super::TransportChecksumError::Icmpv6InIpv4 => BuildWriteError::Icmpv6InIpv4,
}
}
}

#[cfg(feature = "std")]
impl From<crate::WriteError<std::io::Error, ipv4_exts::ExtsWalkError>> for BuildWriteError {
fn from(value: crate::WriteError<std::io::Error, ipv4_exts::ExtsWalkError>) -> Self {
match value {
crate::WriteError::Io(err) => BuildWriteError::Io(err),
crate::WriteError::Content(err) => BuildWriteError::Ipv4Exts(err),
}
}
}

#[cfg(feature = "std")]
impl From<crate::WriteError<std::io::Error, ipv6_exts::ExtsWalkError>> for BuildWriteError {
fn from(value: crate::WriteError<std::io::Error, ipv6_exts::ExtsWalkError>) -> Self {
match value {
crate::WriteError::Io(err) => BuildWriteError::Io(err),
crate::WriteError::Content(err) => BuildWriteError::Ipv6Exts(err),
}
}
}

#[cfg(test)]
mod tests {
use super::{BuildWriteError::*, *};
Expand Down
8 changes: 8 additions & 0 deletions etherparse/src/err/packet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ mod build_write_error;
#[cfg(feature = "std")]
pub use build_write_error::*;

#[cfg(feature = "alloc")]
mod build_vec_write_error;
#[cfg(feature = "alloc")]
pub use build_vec_write_error::*;

mod build_slice_write_error;
pub use build_slice_write_error::*;

mod slice_error;
pub use slice_error::*;

Expand Down
7 changes: 4 additions & 3 deletions etherparse/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@
// for docs.rs
#![cfg_attr(docsrs, feature(doc_cfg))]

#[cfg(test)]
#[cfg(any(feature = "alloc", test))]
extern crate alloc;
#[cfg(test)]
extern crate proptest;
Expand Down Expand Up @@ -343,6 +343,9 @@ mod compositions_tests;
mod helpers;
pub(crate) use helpers::*;

mod writer;
pub(crate) use writer::*;

mod lax_packet_headers;
pub use lax_packet_headers::*;

Expand All @@ -358,9 +361,7 @@ pub(crate) use lax_sliced_packet_cursor::*;
mod len_source;
pub use len_source::*;

#[cfg(feature = "std")]
mod packet_builder;
#[cfg(feature = "std")]
pub use crate::packet_builder::*;

mod packet_headers;
Expand Down
27 changes: 20 additions & 7 deletions etherparse/src/net/ipv4_exts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,21 +160,19 @@ impl Ipv4Extensions {
}

/// Write the extensions to the writer.
#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn write<T: std::io::Write + Sized>(
pub(crate) fn write_internal<T: CoreWrite + ?Sized>(
&self,
writer: &mut T,
start_ip_number: IpNumber,
) -> Result<(), err::ipv4_exts::HeaderWriteError> {
use err::ipv4_exts::{ExtsWalkError::*, HeaderWriteError::*};
) -> Result<(), WriteError<T::Error, err::ipv4_exts::ExtsWalkError>> {
use err::ipv4_exts::ExtsWalkError::*;
use ip_number::*;
match self.auth {
Some(ref header) => {
if AUTH == start_ip_number {
header.write(writer).map_err(Io)
writer.write_all(&header.to_bytes()).map_err(WriteError::Io)
} else {
Err(Content(ExtNotReferenced {
Err(WriteError::Content(ExtNotReferenced {
missing_ext: IpNumber::AUTHENTICATION_HEADER,
}))
}
Expand All @@ -183,6 +181,21 @@ impl Ipv4Extensions {
}
}

/// Write the extensions to the writer.
#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn write<T: std::io::Write + Sized>(
&self,
writer: &mut T,
start_ip_number: IpNumber,
) -> Result<(), err::ipv4_exts::HeaderWriteError> {
self.write_internal(&mut IoWriter(writer), start_ip_number)
.map_err(|err| match err {
WriteError::Io(err) => err::ipv4_exts::HeaderWriteError::Io(err),
WriteError::Content(err) => err::ipv4_exts::HeaderWriteError::Content(err),
})
}

///Length of the all present headers in bytes.
pub fn header_len(&self) -> usize {
if let Some(ref header) = self.auth {
Expand Down
Loading
Loading