Skip to content

Commit

Permalink
In functions using ada_owned_string, return ada_owned_string instead …
Browse files Browse the repository at this point in the history
…of &str (#67)

* feat(idna-&-lib/&str->ada_owned_string): change return type to ada_owned_string
ada_owned_string is supposed to be cleared manually. This was not being done currently. Instead an internal reference to the memory was being returned by these functions. This led to us being unable to delete the memory out of fear of invalidating the &str present in user space. By returning ada_owned_string, this issue is resolved and the memory leaks no longer happen in library space. Old &str can still be accessed using as_ref.

BREAKING CHANGE: Functions using ada_owned_string now return ada_owned_string instead of &str. Memory leaks no longer happen in library space. Old &str can still be accessed using as_ref.

* feat(ada_owned_string/Drop): add
Clears the memory after the ada_owned_string goes out of scope. This ensures that ada_owned_string is cleared in user space.

BREAKING CHANGE: ada_owned_string was previously not freed upon going out of scope. Now it is.

* feat(ada_owned_string/ToString): add

Makes it possible to convert ada_owned_string to ToString

* refactor(ada_owned_string->String): change return type from ada_owned_string to String

Ensures that ada_owned_string does not leak to user space and allows us to make internal breaking changes to its implementation. Fixes all leak issue. Causes small performance downgrades due to copies. Causes functions to be usable only if std is enabled.

BREAKING CHANGE: Return type changed from ada_owned_string->String. Functions now work only with std feature enabled.
  • Loading branch information
pratikpc authored May 20, 2024
1 parent 57a3179 commit f1bf053
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 18 deletions.
23 changes: 23 additions & 0 deletions src/ffi.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#![allow(non_camel_case_types)]
use core::ffi::{c_char, c_uint};

#[cfg(feature = "std")]
extern crate std;

#[repr(C)]
pub struct ada_url {
_unused: [u8; 0],
Expand Down Expand Up @@ -38,6 +41,26 @@ impl AsRef<str> for ada_owned_string {
}
}

#[cfg(feature = "std")]
impl ToString for ada_owned_string {
fn to_string(&self) -> std::string::String {
self.as_ref().to_owned()
}
}

impl Drop for ada_owned_string {
fn drop(&mut self) {
// @note This is needed because ada_free_owned_string accepts by value
let copy = ada_owned_string {
data: self.data,
length: self.length,
};
unsafe {
ada_free_owned_string(copy);
};
}
}

#[repr(C)]
pub struct ada_url_components {
pub protocol_end: u32,
Expand Down
28 changes: 16 additions & 12 deletions src/idna.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
#[cfg(feature = "std")]
extern crate std;

#[cfg_attr(not(feature = "std"), allow(unused_imports))]
use crate::ffi;

#[cfg(feature = "std")]
use std::string::String;

/// IDNA struct implements the `to_ascii` and `to_unicode` functions from the Unicode Technical
/// Standard supporting a wide range of systems. It is suitable for URL parsing.
/// For more information, [read the specification](https://www.unicode.org/reports/tr46/#ToUnicode)
Expand All @@ -16,12 +23,9 @@ impl Idna {
/// assert_eq!(Idna::unicode("xn--meagefactory-m9a.ca"), "meßagefactory.ca");
/// ```
#[must_use]
pub fn unicode(input: &str) -> &str {
unsafe {
let out = ffi::ada_idna_to_unicode(input.as_ptr().cast(), input.len());
let slice = core::slice::from_raw_parts(out.data.cast(), out.length);
core::str::from_utf8_unchecked(slice)
}
#[cfg(feature = "std")]
pub fn unicode(input: &str) -> String {
unsafe { ffi::ada_idna_to_unicode(input.as_ptr().cast(), input.len()) }.to_string()
}

/// Process international domains according to the UTS #46 standard.
Expand All @@ -34,26 +38,26 @@ impl Idna {
/// assert_eq!(Idna::ascii("meßagefactory.ca"), "xn--meagefactory-m9a.ca");
/// ```
#[must_use]
pub fn ascii(input: &str) -> &str {
unsafe {
let out = ffi::ada_idna_to_ascii(input.as_ptr().cast(), input.len());
let slice = core::slice::from_raw_parts(out.data.cast(), out.length);
core::str::from_utf8_unchecked(slice)
}
#[cfg(feature = "std")]
pub fn ascii(input: &str) -> String {
unsafe { ffi::ada_idna_to_ascii(input.as_ptr().cast(), input.len()) }.to_string()
}
}

#[cfg(test)]
mod tests {
#[cfg_attr(not(feature = "std"), allow(unused_imports))]
use crate::idna::*;

#[test]
fn unicode_should_work() {
#[cfg(feature = "std")]
assert_eq!(Idna::unicode("xn--meagefactory-m9a.ca"), "meßagefactory.ca");
}

#[test]
fn ascii_should_work() {
#[cfg(feature = "std")]
assert_eq!(Idna::ascii("meßagefactory.ca"), "xn--meagefactory-m9a.ca");
}
}
18 changes: 12 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ pub mod ffi;
mod idna;
pub use idna::Idna;

#[cfg(feature = "std")]
extern crate std;

#[cfg(feature = "std")]
use std::string::String;

use core::{borrow, ffi::c_uint, fmt, hash, ops};
use derive_more::Display;

Expand Down Expand Up @@ -270,12 +276,9 @@ impl Url {
/// assert_eq!(url.origin(), "https://example.com");
/// ```
#[must_use]
pub fn origin(&self) -> &str {
unsafe {
let out = ffi::ada_get_origin(self.0);
let slice = core::slice::from_raw_parts(out.data.cast(), out.length);
core::str::from_utf8_unchecked(slice)
}
#[cfg(feature = "std")]
pub fn origin(&self) -> String {
unsafe { ffi::ada_get_origin(self.0) }.to_string()
}

/// Return the parsed version of the URL with all components.
Expand Down Expand Up @@ -949,7 +952,10 @@ mod test {
None,
)
.expect("Should have parsed a simple url");

#[cfg(feature = "std")]
assert_eq!(out.origin(), "https://google.com:9090");

assert_eq!(
out.href(),
"https://username:[email protected]:9090/search?query#hash"
Expand Down

0 comments on commit f1bf053

Please sign in to comment.