Skip to content
Open
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
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Result<T> = core::result::Result<T, TlvError>;
pub enum TlvError {
TruncatedTlv,
InvalidLength,
InvalidTagNumber,
InvalidTagNumber { is_zero: bool },
TooShortBody { expected: usize, found: usize },
Comment on lines 38 to 42
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Renaming InvalidTagNumber from a unit variant to InvalidTagNumber { is_zero: bool } is a public API breaking change for downstream code that pattern-matches on TlvError. Since is_zero is only used internally to detect/record leading padding bytes (and isn’t reflected in the Display message), consider handling that case without encoding internal details into the public error type (e.g., by peeking/consuming 0x00 bytes in the parser loop, or by using a private/internal error and mapping to InvalidTagNumber).

Copilot uses AI. Check for mistakes.
ValExpected { tag_number: usize },
TagPathError,
Expand All @@ -53,7 +53,7 @@ impl fmt::Display for TlvError {
match self {
TruncatedTlv => write!(f, "Too short input vector"),
InvalidLength => writeln!(f, "Invalid length value"),
InvalidTagNumber => write!(f, "Invalid tag number"),
InvalidTagNumber { .. } => write!(f, "Invalid tag number"),
TooShortBody { expected, found } => {
write!(f, "Too short body: expected {expected}, found {found}")
}
Expand Down
60 changes: 44 additions & 16 deletions src/tlv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub enum Value {
pub struct Tlv {
tag: Tag,
val: Value,
leading_zeroes: usize,
}

impl Tlv {
Expand Down Expand Up @@ -45,7 +46,7 @@ impl Tlv {
/// # assert_eq!(constructed_tlv.to_vec(), vec![0x21, 0x02, 0x01, 0x00]);
/// ```
pub fn new(tag: Tag, value: Value) -> Result<Tlv> {
let mut tlv = Tlv { tag, val: value };
let mut tlv = Tlv { tag, val: value, leading_zeroes: 0 };
let is_tlv_primitive = tlv.is_primitive();
tlv.val = match tlv.val {
Value::TlvList(list) => {
Expand Down Expand Up @@ -140,12 +141,14 @@ impl Tlv {
/// assert_eq!(tlv.to_vec(), vec![0x21, 0x08, 0x01, 0x02, 0xA1, 0xA2, 0x02, 0x02, 0xB1, 0xB2]);
/// ```
pub fn to_vec(&self) -> Vec<u8> {
let mut out: Vec<u8> = (self.tag as u64)
let mut out: Vec<u8> = vec![0; self.leading_zeroes];

out.append(&mut (self.tag as u64)
.to_be_bytes()
.iter()
.skip_while(|&&x| x == 0)
.cloned()
.collect();
.collect());

out.append(&mut self.val.encode_len());
Comment on lines 143 to 153
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

to_vec() now prepends leading_zeroes, but Tlv::len()/Value::len() (used by encode_len() and by Tlv::new() when splitting a constructed Value::Val) do not account for those bytes. This makes constructed TLVs emit an incorrect length field and can break re-parsing/indexing when a child TLV has leading zeroes. Include leading_zeroes in the encoded-size calculation (and any other length math) or move the preserved padding out of the TLV byte stream produced by to_vec() so length remains consistent.

Copilot uses AI. Check for mistakes.

Expand Down Expand Up @@ -236,22 +239,15 @@ impl Tlv {
fn read_tag(iter: &mut dyn ExactSizeIterator<Item = &u8>) -> Result<Tag> {
let mut tag: usize;

// Per EMV 4.3 Book 3 Annex B1 (Coding of the Tag Field of BER-TLV Data Objects):
// > Before, between, or after TLV-coded data objects, '00' bytes without any meaning
// > may occur (for example, due to erased or modified TLV-coded data objects).
let first: u8 = iter
.skip_while(|&&next| next == 0)
.next()
.cloned()
.ok_or_else(|| TlvError::TruncatedTlv)?;
let first: u8 = iter.next().cloned().ok_or_else(|| TlvError::TruncatedTlv)?;
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

read_tag() no longer skips leading 0x00 bytes (previously it did). from_iter() now re-implements skipping/counting, but other callers like parse_tag_list() still call read_tag() directly, so their behavior changes to error on padding zeros. If padding zeros should still be tolerated there, consider keeping a “skip 0x00” variant of tag parsing for non-roundtrip use cases, or move the zero-skipping logic back into read_tag() behind a mode/flag.

Suggested change
let first: u8 = iter.next().cloned().ok_or_else(|| TlvError::TruncatedTlv)?;
// Skip leading 0x00 bytes (padding) before reading the actual tag.
let first: u8 = loop {
let byte = iter.next().cloned().ok_or_else(|| TlvError::TruncatedTlv)?;
if byte == 0x00 {
continue;
} else {
break byte;
}
};

Copilot uses AI. Check for mistakes.
tag = first as usize;

if first & 0x1F == 0x1F {
// long form - find the end
for x in &mut *iter {
tag = tag
.checked_shl(8)
.ok_or_else(|| TlvError::InvalidTagNumber)?;
.ok_or_else(|| TlvError::InvalidTagNumber { is_zero: false })?;

tag |= *x as usize;

Expand All @@ -262,7 +258,7 @@ impl Tlv {
}

if tag == 0 {
return Err(TlvError::InvalidTagNumber);
return Err(TlvError::InvalidTagNumber { is_zero: true });
}

Ok(tag)
Expand Down Expand Up @@ -303,14 +299,30 @@ impl Tlv {

/// Initializes Tlv object iterator of Vec<u8>
fn from_iter(iter: &mut dyn ExactSizeIterator<Item = &u8>) -> Result<Tlv> {
let tag = Tlv::read_tag(iter)?;
// Per EMV 4.3 Book 3 Annex B1 (Coding of the Tag Field of BER-TLV Data Objects):
// > Before, between, or after TLV-coded data objects, '00' bytes without any meaning
// > may occur (for example, due to erased or modified TLV-coded data objects).
let (tag, leading_zeroes) = {
let mut leading_zeroes: usize = 0;
let tag = loop {
match Tlv::read_tag(iter) {
Ok(x) => break x,
Err(TlvError::InvalidTagNumber { is_zero: true }) => {
leading_zeroes += 1;
}
Err(x) => return Err(x),
}
};
(tag, leading_zeroes)
};
let len = Tlv::read_len(iter)?;

let val = &mut iter.take(len);

let mut tlv = Tlv {
tag,
val: Value::Nothing,
leading_zeroes,
};

if tlv.is_primitive() {
Expand Down Expand Up @@ -519,14 +531,14 @@ mod tests {
let input: Vec<u8> = vec![0x00, 0x00, 0x01, 0x02, 0x00, 0x00];
assert_eq!(
Tlv::from_vec(&input).unwrap().to_vec(),
vec![0x01, 0x02, 0x00, 0x00]
vec![0x00, 0x00, 0x01, 0x02, 0x00, 0x00]
);

// Constructed TLV that contains two primitive TLVs with zeroes between them
let input: Vec<u8> = vec![0xE1, 0x08, 0x01, 0x01, 0x01, 0x00, 0x00, 0x02, 0x01, 0x02];
assert_eq!(
Tlv::from_vec(&input).unwrap().to_vec(),
vec![0xE1, 0x06, 0x01, 0x01, 0x01, 0x02, 0x01, 0x02]
vec![0xE1, 0x06, 0x01, 0x01, 0x01, 0x00, 0x00, 0x02, 0x01, 0x02]
Comment thread
mrnerdhair marked this conversation as resolved.
);
}

Expand All @@ -535,34 +547,39 @@ mod tests {
let tlv = Tlv {
tag: 0x01,
val: Value::Val(vec![0]),
leading_zeroes: 0,
};

assert_eq!(tlv.to_vec(), vec![0x01, 0x01, 0x00]);

let tlv = Tlv {
tag: 0x01,
val: Value::Val(vec![0; 127]),
leading_zeroes: 0,
};

assert_eq!(&tlv.to_vec()[0..3], [0x01, 0x7F, 0x00]);

let tlv = Tlv {
tag: 0x01,
val: Value::Val(vec![0; 255]),
leading_zeroes: 0,
};

assert_eq!(&tlv.to_vec()[0..4], [0x01, 0x81, 0xFF, 0x00]);

let tlv = Tlv {
tag: 0x02,
val: Value::Val(vec![0; 256]),
leading_zeroes: 0,
};

assert_eq!(&tlv.to_vec()[0..4], [0x02, 0x82, 0x01, 0x00]);

let tlv = Tlv {
tag: 0x03,
val: Value::Val(vec![0; 0xffff01]),
leading_zeroes: 0,
};

assert_eq!(&tlv.to_vec()[0..5], [0x03, 0x83, 0xFF, 0xFF, 0x01]);
Expand All @@ -585,18 +602,22 @@ mod tests {
let tlv1 = Tlv {
tag: 0x03,
val: Value::Nothing,
leading_zeroes: 0,
};
let tlv2 = Tlv {
tag: 0x0303,
val: Value::Nothing,
leading_zeroes: 0,
};
let tlv3 = Tlv {
tag: 0x030303,
val: Value::Nothing,
leading_zeroes: 0,
};
let tlv4 = Tlv {
tag: 0x03030303,
val: Value::Nothing,
leading_zeroes: 0,
};

assert_eq!(tlv1.tag_len(), 1);
Expand All @@ -615,17 +636,21 @@ mod tests {
Tlv {
tag: 1,
val: Value::Nothing,
leading_zeroes: 0,
},
Tlv {
tag: 2,
val: Value::Val(vec![1, 2, 3]),
leading_zeroes: 0,
},
Tlv {
tag: 3,
val: Value::TlvList(vec![Tlv {
tag: 4,
val: Value::Nothing,
leading_zeroes: 0,
}]),
leading_zeroes: 0,
},
])
.to_vec(),
Expand Down Expand Up @@ -655,14 +680,17 @@ mod tests {
Tlv {
tag: 0x01,
val: Value::Val(vec![0x01]),
leading_zeroes: 0,
},
Tlv {
tag: 0x02,
val: Value::Val(vec![0x02, 0x02]),
leading_zeroes: 0,
},
Tlv {
tag: 0x03,
val: Value::Val(vec![0x03, 0x03, 0x03]),
leading_zeroes: 0,
}
])
)
Expand Down
9 changes: 2 additions & 7 deletions tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,9 @@ fn quickcheck_from_vec() {
match tlv::Tlv::from_vec(&xs) {
Ok(tlv) => {
let restored_tlv = tlv.to_vec();
let truncated_xs = xs.into_iter().take(restored_tlv.len()).collect::<Vec<u8>>();

let truncacted_xs = xs
.into_iter()
.skip_while(|&x| x == 0)
.take(restored_tlv.len())
.collect::<Vec<u8>>();

TestResult::from_bool(restored_tlv == truncacted_xs)
TestResult::from_bool(restored_tlv == truncated_xs)
}
Err(_) => TestResult::discard(),
}
Expand Down