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

Add ability to convert errors to wire_error_code via C API #1762

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions quiche/include/quiche.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ enum quiche_error {
// Returns a human readable string with the quiche version number.
const char *quiche_version(void);

// Returns the wire error code for the given error as defined in RFC9000 Section 20. Error Codes.
// The given error must be a valid quiche_error.
uint64_t quiche_error_to_wire_error_code(ssize_t error);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ghedo @LPardue I only added the code for the quic errors for now but if you agree with the approach let me know and I will add the same for h3.

One thing I was not sure about was if using a ssize_t is good or if we should use quiche_error and expose it via FFI as enum.

Copy link
Member

Choose a reason for hiding this comment

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

TBH it's not really clear why an application would want this? The 1d00ee1 change simply added public enums for the wire error codes defined in RFCs, the conversion from Error is not actually public, so this FFI change would add an API that is not actually available in Rust.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because an app might want to hide the fact that they use quiche underneath and just return the "RFC" error codes. That said it's easy for me to workaround it.


// Enables logging. |cb| will be called with log messages
int quiche_enable_debug_logging(void (*cb)(const char *line, void *argp),
void *argp);
Expand Down
5 changes: 5 additions & 0 deletions quiche/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ pub extern fn quiche_version() -> *const u8 {
VERSION.as_ptr()
}

#[no_mangle]
pub extern fn quiche_error_to_wire_error_code(error: ssize_t) -> u64 {
Error::from_c(error).to_wire()
}

struct Logger {
cb: extern fn(line: *const u8, argp: *mut c_void),
argp: std::sync::atomic::AtomicPtr<c_void>,
Expand Down
33 changes: 33 additions & 0 deletions quiche/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,39 @@ impl Error {
Error::CryptoBufferExceeded => -20,
}
}

#[cfg(feature = "ffi")]
fn from_c(error: libc::ssize_t) -> Error {
match error {
-1 => Error::Done,
-2 => Error::BufferTooShort,
-3 => Error::UnknownVersion,
-4 => Error::InvalidFrame,
-5 => Error::InvalidPacket,
-6 => Error::InvalidState,
// Let's use a value of 0 as a placeholder as we only use it in the
// FFI code and so don't care about it.
-7 => Error::InvalidStreamState(0),
-8 => Error::InvalidTransportParam,
-9 => Error::CryptoFail,
-10 => Error::TlsFail,
-11 => Error::FlowControl,
-12 => Error::StreamLimit,
-13 => Error::FinalSize,
-14 => Error::CongestionControl,
// Let's use a value of 0 as a placeholder as we only use it in the
// FFI code and so don't care about it.
-15 => Error::StreamStopped(0),
// Let's use a value of 0 as a placeholder as we only use it in the
// FFI code and so don't care about it.
-16 => Error::StreamReset(0),
-17 => Error::IdLimit,
-18 => Error::OutOfIdentifiers,
-19 => Error::KeyUpdate,
-20 => Error::CryptoBufferExceeded,
_ => panic!("expected error {:?}", error),
}
}
}

impl std::fmt::Display for Error {
Expand Down