Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support arbitrary byte sequence containers #3

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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 Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ documentation = "https://docs.rs/iso7816"

[dependencies]
delog = "0.1.2"
heapless = "0.7"
# use unreleased heapless for https://github.com/japaric/heapless/pull/255
heapless = { git = "https://github.com/japaric/heapless" }
Copy link
Member

Choose a reason for hiding this comment

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

Seems this is merged, maybe also published?


[dev-dependencies]
hex-literal = "0.3.1"
35 changes: 18 additions & 17 deletions src/command.rs
Original file line number Diff line number Diff line change
@@ -1,35 +1,36 @@
use core::convert::TryFrom;

use crate::Data;
use crate::somebytes::{Bytes, TryExtendFromSlice};

pub mod class;
pub mod instruction;
pub use instruction::Instruction;

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Command<const S: usize>
pub struct Command<B: AsRef<[u8]>>
{
class: class::Class,
instruction: Instruction,

pub p1: u8,
pub p2: u8,

/// The main reason this is modeled as Bytes and not
/// a fixed array is for serde purposes.
data: Data<S>,
data: Bytes<B>,

le: usize,
pub extended: bool,
}

impl<const S: usize> Command<S>
impl<'a, B: AsRef<[u8]> + TryFrom<&'a [u8]>> Command<B>
{
pub fn try_from(apdu: &[u8]) -> Result<Self, FromSliceError> {
pub fn try_from(apdu: &'a [u8]) -> Result<Self, FromSliceError> {
use core::convert::TryInto;
apdu.try_into()
}
}

impl<B: AsRef<[u8]>> Command<B>
{
pub fn class(&self) -> class::Class {
self.class
}
Expand All @@ -38,22 +39,24 @@ impl<const S: usize> Command<S>
self.instruction
}

pub fn data(&self) -> &Data<S> {
pub fn data(&self) -> &B {
&self.data
}

pub fn data_mut(&mut self) -> &mut Data<S> {
pub fn data_mut(&mut self) -> &mut B {
&mut self.data
}

pub fn expected(&self) -> usize {
self.le
}
}

impl<B: AsRef<[u8]> + TryExtendFromSlice<u8>> Command<B> {
/// This can be use for APDU chaining to convert
/// multiple APDU's into one.
/// * Global Platform GPC_SPE_055 3.10
pub fn extend_from_command<const T: usize>(&mut self, command: &Command<T>) -> core::result::Result<(), ()> {
pub fn extend_from_command<T: AsRef<[u8]>>(&mut self, command: &Command<T>) -> core::result::Result<(), B::Error> {

// Always take the header from the last command;
self.class = command.class();
Expand All @@ -64,7 +67,7 @@ impl<const S: usize> Command<S>
self.extended = true;

// add the data to the end.
self.data.extend_from_slice(&command.data())
self.data.extend_from_slice(command.data().as_ref())
}
}

Expand All @@ -83,10 +86,9 @@ impl From<class::InvalidClass> for FromSliceError {
}
}

impl<const S: usize> TryFrom<&[u8]> for Command<S>
{
impl<'a, B: AsRef<[u8]> + TryFrom<&'a [u8]>> TryFrom<&'a [u8]> for Command<B> {
type Error = FromSliceError;
fn try_from(apdu: &[u8]) -> core::result::Result<Self, Self::Error> {
fn try_from(apdu: &'a [u8]) -> core::result::Result<Self, Self::Error> {
if apdu.len() < 4 {
return Err(FromSliceError::TooShort);
}
Expand All @@ -106,8 +108,7 @@ impl<const S: usize> TryFrom<&[u8]> for Command<S>
// maximum expected response length
le: parsed.le,
// payload
data: Data::from_slice(data_slice)
.map_err(|_| Self::Error::TooLong)?,
data: B::try_from(data_slice).map_err(|_| Self::Error::TooLong)?.into(),
extended: parsed.extended,
})
}
Expand Down Expand Up @@ -249,6 +250,6 @@ mod test {
0x06, 0x03, 0x55, 0x1d,
];

let command = Command::<256>::try_from(apdu).unwrap();
let command = Command::<heapless::Vec::<u8, 256>>::try_from(apdu).unwrap();
}
}
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ pub enum Interface {
Contactless,
}

pub type Data<const S: usize> = heapless::Vec<u8, S>;
Copy link
Member

Choose a reason for hiding this comment

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

pub use data::Data (where data::Data is the current somebytes::Bytes)? There's already a bunch of Bytes types in the eco-system, if we're rolling our own it would seem to make sense to name it semantically.

pub type Result<T=()> = core::result::Result<T, Status>;

pub mod aid;
pub mod command;
pub mod response;
pub mod somebytes;
Copy link
Member

Choose a reason for hiding this comment

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

Rename to data?

Copy link
Member

Choose a reason for hiding this comment

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

Also generally wondering if we should have all these public submodules; assuming they're just implementation, we could pub use the core data structures + traits at crate level and let the submodules be non-public.


pub use aid::{Aid, App};
pub use command::{Command, Instruction};
Expand Down
8 changes: 4 additions & 4 deletions src/response.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
mod status;
pub use status::Status;

use crate::Data;
use crate::somebytes::Bytes;

#[derive(Clone, Debug, Eq, PartialEq)]
pub enum Response<const S: usize> {
Data(Data<S>),
pub enum Response<B: AsRef<[u8]>> {
Data(Bytes<B>),
Status(Status),
}

impl<const S: usize> Default for Response<S> {
impl<B: AsRef<[u8]>> Default for Response<B> {
fn default() -> Self {
Self::Status(Default::default())
}
Expand Down
9 changes: 3 additions & 6 deletions src/response/status.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use core::convert::TryFrom;
use crate::Data;

impl Default for Status {
fn default() -> Self {
Expand Down Expand Up @@ -171,12 +170,10 @@ impl Into<[u8; 2]> for Status {
}
}

impl<const S: usize> Into<Data<S>> for Status
{
impl<const S: usize> Into<heapless::Vec<u8, S>> for Status {
#[inline]
fn into(self) -> Data<S> {
fn into(self) -> heapless::Vec<u8, S> {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add an inherent into_vec method (in case it doesn't already exist).

let arr: [u8; 2] = self.into();
Data::from_slice(&arr).unwrap()
heapless::Vec::from_slice(&arr).unwrap()
}
}

79 changes: 79 additions & 0 deletions src/somebytes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// This could be a separate crate.

use core::{cmp, fmt, ops};

pub trait TryExtendFromSlice<T> {
type Error;
fn extend_from_slice(&mut self, slice: &[T]) -> Result<(), Self::Error>;
}

impl<T: Clone, const N: usize> TryExtendFromSlice<T> for heapless::Vec<T, N> {
Copy link
Member

@nickray nickray Jun 1, 2022

Choose a reason for hiding this comment

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

I think it would make sense to have a alloc feature on the crate, and conditionally implement TryExtendFromSlice for alloc::vec::Vec?

type Error = ();
fn extend_from_slice(&mut self, slice: &[T]) -> Result<(), Self::Error> {
heapless::Vec::extend_from_slice(self, slice)
}
}

/// A wrapper type for a byte sequence.
///
/// This wrapper implements common traits based on the content of the byte sequence.
#[derive(Clone)]
pub struct Bytes<T: AsRef<[u8]>>(T);
Copy link
Member

Choose a reason for hiding this comment

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

I suggest pub struct Data<B: AsRef<[u8]>>(B) here. You're also using the B elsewhere (pointing to it meaning Bytes).


impl<T: AsRef<[u8]>> Bytes<T> {
pub fn into_bytes(self) -> T {
self.0
}
}

impl<T: AsRef<[u8]>> AsRef<[u8]> for Bytes<T> {
fn as_ref(&self) -> &[u8] {
self.0.as_ref()
}
}

impl<T: AsRef<[u8]>> ops::Deref for Bytes<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T: AsRef<[u8]>> ops::DerefMut for Bytes<T> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl<T: AsRef<[u8]>> From<T> for Bytes<T> {
fn from(bytes: T) -> Self {
Self(bytes)
}
}

impl<T: AsRef<[u8]>> fmt::Debug for Bytes<T> {
Copy link
Member

@nickray nickray Jun 1, 2022

Choose a reason for hiding this comment

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

Might be nice to have the Debug display hex? We already depend on delog, which re-implements hex-fmt.

Not necessarily for this PR, but it would be nice to have a serde feature, with at least a corresponding serde::ser implementation (as this can't be done outside this crate). Even nicer if it outputs hex if serializer.is_human_readable(), else actual bytes.

fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.0.as_ref().fmt(f)
}
}

impl<T: AsRef<[u8]>> Eq for Bytes<T> {}

impl<T: AsRef<[u8]>> PartialEq for Bytes<T> {
fn eq(&self, other: &Self) -> bool {
self.0.as_ref().eq(other.0.as_ref())
}
}

impl<T: AsRef<[u8]>> Ord for Bytes<T> {
fn cmp(&self, other: &Self) -> cmp::Ordering {
self.0.as_ref().cmp(other.0.as_ref())
}
}

impl<T: AsRef<[u8]>> PartialOrd for Bytes<T> {
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
self.0.as_ref().partial_cmp(other.0.as_ref())
}
}