jid: Refactor all three JID types

The main reason for this refactor was to make common operations simpler,
for instance formatting a JID is now a simple clone of a String.

Instead of having three different String for each of node, domain and
resource, we now have a single String with offsets pointing to where the
at and slash are (if they are present).

This also reduces the size of a FullJid from 72 bytes to 32 bytes on
64-bit platforms (less so on 32-bit), and BareJid from 48 bytes to
32 bytes.  Jid is still 40 bytes instead of 32, but that can be improved
in a future version where InnerJid has been inlined into each struct.

jid/Cargo.toml

@@ -19,6 +19,7 @@ edition = "2018"
 gitlab = { repository = "xmpp-rs/xmpp-rs" }
 
 [dependencies]
+memchr = "2.5"
 minidom = { version = "0.15", optional = true }
 serde = { version = "1.0", features = ["derive"], optional = true }
 stringprep = "0.1.2"

jid/src/error.rs

@@ -12,7 +12,7 @@ use std::error::Error as StdError;
 use std::fmt;
 
 /// An error that signifies that a `Jid` cannot be parsed from a string.
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq)]
 pub enum JidParseError {
     /// Happens when there is no domain, that is either the string is empty,
     /// starts with a /, or contains the @/ sequence.
@@ -27,47 +27,44 @@ pub enum JidParseError {
     /// Happens when the resource is empty, that is the string ends with a /.
     EmptyResource,
 
-    /// Happens when the JID is invalid according to stringprep. TODO: make errors
+    /// Happens when the localpart is longer than 1023 bytes.
-    /// meaningful.
+    NodeTooLong,
-    Stringprep(stringprep::Error),
-}
 
-impl From<stringprep::Error> for JidParseError {
+    /// Happens when the domain is longer than 1023 bytes.
-    fn from(e: stringprep::Error) -> JidParseError {
+    DomainTooLong,
-        JidParseError::Stringprep(e)
-    }
-}
 
-impl PartialEq for JidParseError {
+    /// Happens when the resource is longer than 1023 bytes.
-    fn eq(&self, other: &JidParseError) -> bool {
+    ResourceTooLong,
-        use JidParseError as E;
-        match (self, other) {
-            (E::NoDomain, E::NoDomain) => true,
-            (E::NoResource, E::NoResource) => true,
-            (E::EmptyNode, E::EmptyNode) => true,
-            (E::EmptyResource, E::EmptyResource) => true,
-            (E::Stringprep(_), E::Stringprep(_)) => true, // TODO: fix that upstream.
-            _ => false,
-        }
-    }
-}
 
-impl Eq for JidParseError {}
+    /// Happens when the localpart is invalid according to nodeprep.
+    NodePrep,
+
+    /// Happens when the domain is invalid according to nameprep.
+    NamePrep,
+
+    /// Happens when the resource is invalid according to resourceprep.
+    ResourcePrep,
+
+    /// Happens when parsing a bare JID and there is a resource.
+    ResourceInBareJid,
+}
 
 impl StdError for JidParseError {}
 
 impl fmt::Display for JidParseError {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(
+        fmt.write_str(match self {
-            fmt,
+            JidParseError::NoDomain => "no domain found in this JID",
-            "{}",
+            JidParseError::NoResource => "no resource found in this full JID",
-            match self {
+            JidParseError::EmptyNode => "nodepart empty despite the presence of a @",
-                JidParseError::NoDomain => "no domain found in this JID",
+            JidParseError::EmptyResource => "resource empty despite the presence of a /",
-                JidParseError::NoResource => "no resource found in this full JID",
+            JidParseError::NodeTooLong => "localpart longer than 1023 bytes",
-                JidParseError::EmptyNode => "nodepart empty despite the presence of a @",
+            JidParseError::DomainTooLong => "domain longer than 1023 bytes",
-                JidParseError::EmptyResource => "resource empty despite the presence of a /",
+            JidParseError::ResourceTooLong => "resource longer than 1023 bytes",
-                JidParseError::Stringprep(_err) => "TODO",
+            JidParseError::NodePrep => "localpart doesn’t pass nodeprep validation",
-            }
+            JidParseError::NamePrep => "domain doesn’t pass nameprep validation",
-        )
+            JidParseError::ResourcePrep => "resource doesn’t pass resourceprep validation",
+            JidParseError::ResourceInBareJid => "resource found while parsing a bare JID",
+        })
     }
 }

jid/src/inner.rs

@@ -0,0 +1,201 @@
+// Copyright (c) 2023 Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#![deny(missing_docs)]
+
+//! Provides a type for Jabber IDs.
+//!
+//! For usage, check the documentation on the `Jid` struct.
+
+use crate::JidParseError as Error;
+use core::num::NonZeroU16;
+use memchr::memchr;
+use std::borrow::Cow;
+use std::str::FromStr;
+use stringprep::{nameprep, nodeprep, resourceprep};
+
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub(crate) struct InnerJid {
+    pub(crate) normalized: String,
+    pub(crate) at: Option<NonZeroU16>,
+    pub(crate) slash: Option<NonZeroU16>,
+}
+
+impl InnerJid {
+    pub(crate) fn new(unnormalized: &str) -> Result<InnerJid, Error> {
+        let bytes = unnormalized.as_bytes();
+        let mut orig_at = memchr(b'@', bytes);
+        let mut orig_slash = memchr(b'/', bytes);
+        if orig_at.is_some() && orig_slash.is_some() && orig_at > orig_slash {
+            // This is part of the resource, not a node@domain separator.
+            orig_at = None;
+        }
+
+        let normalized = match (orig_at, orig_slash) {
+            (Some(at), Some(slash)) => {
+                let node = nodeprep(&unnormalized[..at]).map_err(|_| Error::NodePrep)?;
+                if node.len() == 0 {
+                    return Err(Error::EmptyNode);
+                }
+                if node.len() > 1023 {
+                    return Err(Error::NodeTooLong);
+                }
+                let domain = nameprep(&unnormalized[at + 1..slash]).map_err(|_| Error::NamePrep)?;
+                if domain.len() == 0 {
+                    return Err(Error::NoDomain);
+                }
+                if domain.len() > 1023 {
+                    return Err(Error::DomainTooLong);
+                }
+                let resource =
+                    resourceprep(&unnormalized[slash + 1..]).map_err(|_| Error::ResourcePrep)?;
+                if resource.len() == 0 {
+                    return Err(Error::EmptyResource);
+                }
+                if resource.len() > 1023 {
+                    return Err(Error::ResourceTooLong);
+                }
+                match (node, domain, resource) {
+                    (Cow::Borrowed(_), Cow::Borrowed(_), Cow::Borrowed(_)) => {
+                        unnormalized.to_owned()
+                    }
+                    (node, domain, resource) => {
+                        orig_at = Some(node.len());
+                        orig_slash = Some(node.len() + domain.len() + 1);
+                        format!("{node}@{domain}/{resource}")
+                    }
+                }
+            }
+            (Some(at), None) => {
+                let node = nodeprep(&unnormalized[..at]).map_err(|_| Error::NodePrep)?;
+                if node.len() == 0 {
+                    return Err(Error::EmptyNode);
+                }
+                if node.len() > 1023 {
+                    return Err(Error::NodeTooLong);
+                }
+                let domain = nameprep(&unnormalized[at + 1..]).map_err(|_| Error::NamePrep)?;
+                if domain.len() == 0 {
+                    return Err(Error::NoDomain);
+                }
+                if domain.len() > 1023 {
+                    return Err(Error::DomainTooLong);
+                }
+                match (node, domain) {
+                    (Cow::Borrowed(_), Cow::Borrowed(_)) => unnormalized.to_owned(),
+                    (node, domain) => {
+                        orig_at = Some(node.len());
+                        format!("{node}@{domain}")
+                    }
+                }
+            }
+            (None, Some(slash)) => {
+                let domain = nameprep(&unnormalized[..slash]).map_err(|_| Error::NamePrep)?;
+                if domain.len() == 0 {
+                    return Err(Error::NoDomain);
+                }
+                if domain.len() > 1023 {
+                    return Err(Error::DomainTooLong);
+                }
+                let resource =
+                    resourceprep(&unnormalized[slash + 1..]).map_err(|_| Error::ResourcePrep)?;
+                if resource.len() == 0 {
+                    return Err(Error::EmptyResource);
+                }
+                if resource.len() > 1023 {
+                    return Err(Error::ResourceTooLong);
+                }
+                match (domain, resource) {
+                    (Cow::Borrowed(_), Cow::Borrowed(_)) => unnormalized.to_owned(),
+                    (domain, resource) => {
+                        orig_slash = Some(domain.len());
+                        format!("{domain}/{resource}")
+                    }
+                }
+            }
+            (None, None) => {
+                let domain = nameprep(unnormalized).map_err(|_| Error::NamePrep)?;
+                if domain.len() == 0 {
+                    return Err(Error::NoDomain);
+                }
+                if domain.len() > 1023 {
+                    return Err(Error::DomainTooLong);
+                }
+                domain.into_owned()
+            }
+        };
+
+        Ok(InnerJid {
+            normalized,
+            at: orig_at.and_then(|x| NonZeroU16::new(x as u16)),
+            slash: orig_slash.and_then(|x| NonZeroU16::new(x as u16)),
+        })
+    }
+
+    pub(crate) fn node(&self) -> Option<&str> {
+        self.at.and_then(|at| {
+            let at = u16::from(at) as usize;
+            Some(&self.normalized[..at])
+        })
+    }
+
+    pub(crate) fn domain(&self) -> &str {
+        match (self.at, self.slash) {
+            (Some(at), Some(slash)) => {
+                let at = u16::from(at) as usize;
+                let slash = u16::from(slash) as usize;
+                &self.normalized[at + 1..slash]
+            }
+            (Some(at), None) => {
+                let at = u16::from(at) as usize;
+                &self.normalized[at + 1..]
+            }
+            (None, Some(slash)) => {
+                let slash = u16::from(slash) as usize;
+                &self.normalized[..slash]
+            }
+            (None, None) => &self.normalized,
+        }
+    }
+
+    pub(crate) fn resource(&self) -> Option<&str> {
+        self.slash.and_then(|slash| {
+            let slash = u16::from(slash) as usize;
+            Some(&self.normalized[slash + 1..])
+        })
+    }
+}
+
+impl FromStr for InnerJid {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        InnerJid::new(s)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    macro_rules! assert_size (
+        ($t:ty, $sz:expr) => (
+            assert_eq!(::std::mem::size_of::<$t>(), $sz);
+        );
+    );
+
+    #[cfg(target_pointer_width = "32")]
+    #[test]
+    fn test_size() {
+        assert_size!(InnerJid, 16);
+    }
+
+    #[cfg(target_pointer_width = "64")]
+    #[test]
+    fn test_size() {
+        assert_size!(InnerJid, 32);
+    }
+}

jid/src/lib.rs

@@ -14,10 +14,11 @@
 //!
 //! For usage, check the documentation on the `Jid` struct.
 
-use std::convert::{Into, TryFrom};
+use core::num::NonZeroU16;
+use std::convert::TryFrom;
 use std::fmt;
 use std::str::FromStr;
-use stringprep::{nameprep, nodeprep, resourceprep};
+use stringprep::resourceprep;
 
 #[cfg(feature = "serde")]
 use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
@@ -25,6 +26,9 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
 mod error;
 pub use crate::error::JidParseError;
 
+mod inner;
+use inner::InnerJid;
+
 /// An enum representing a Jabber ID. It can be either a `FullJid` or a `BareJid`.
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(untagged))]
@@ -40,19 +44,8 @@ pub enum Jid {
 impl FromStr for Jid {
     type Err = JidParseError;
 
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+    fn from_str(s: &str) -> Result<Jid, JidParseError> {
-        let (ns, ds, rs): StringJid = _from_str(s)?;
+        Jid::new(s)
-        Ok(match rs {
-            Some(rs) => Jid::Full(FullJid {
-                node: ns,
-                domain: ds,
-                resource: rs,
-            }),
-            None => Jid::Bare(BareJid {
-                node: ns,
-                domain: ds,
-            }),
-        })
     }
 }
 
@@ -93,26 +86,68 @@ impl fmt::Display for Jid {
 }
 
 impl Jid {
+    /// Constructs a Jabber ID from a string.
+    ///
+    /// This is of the form `node`@`domain`/`resource`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use jid::Jid;
+    /// # use jid::JidParseError;
+    ///
+    /// # fn main() -> Result<(), JidParseError> {
+    /// let jid = Jid::new("node@domain/resource")?;
+    ///
+    /// assert_eq!(jid.node(), Some("node"));
+    /// assert_eq!(jid.domain(), "domain");
+    /// assert_eq!(jid.resource(), Some("resource"));
+    /// # Ok(())
+    /// # }
+    /// ```
+    pub fn new(s: &str) -> Result<Jid, JidParseError> {
+        let inner = InnerJid::new(s)?;
+        if inner.slash.is_some() {
+            Ok(Jid::Full(FullJid { inner }))
+        } else {
+            Ok(Jid::Bare(BareJid { inner }))
+        }
+    }
+
     /// The node part of the Jabber ID, if it exists, else None.
-    pub fn node(self) -> Option<String> {
+    pub fn node(&self) -> Option<&str> {
         match self {
-            Jid::Bare(BareJid { node, .. }) | Jid::Full(FullJid { node, .. }) => node,
+            Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => inner.node(),
         }
     }
 
     /// The domain of the Jabber ID.
-    pub fn domain(self) -> String {
+    pub fn domain(&self) -> &str {
         match self {
-            Jid::Bare(BareJid { domain, .. }) | Jid::Full(FullJid { domain, .. }) => domain,
+            Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => inner.domain(),
         }
     }
-}
 
-impl From<Jid> for BareJid {
+    /// The resource of the Jabber ID.
-    fn from(jid: Jid) -> BareJid {
+    pub fn resource(&self) -> Option<&str> {
-        match jid {
+        match self {
-            Jid::Full(full) => full.into(),
+            Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => inner.resource(),
-            Jid::Bare(bare) => bare,
+        }
+    }
+
+    /// Extract a bare JID from this JID, throwing away the resource.
+    pub fn to_bare(&self) -> BareJid {
+        match self {
+            Jid::Full(jid) => jid.to_bare(),
+            Jid::Bare(jid) => jid.clone(),
+        }
+    }
+
+    /// Transforms this JID into a bare JID, throwing away the resource.
+    pub fn into_bare(self) -> BareJid {
+        match self {
+            Jid::Full(jid) => jid.into_bare(),
+            Jid::Bare(jid) => jid,
         }
     }
 }
@@ -176,12 +211,7 @@ impl PartialEq<BareJid> for Jid {
 /// there is no case where a resource can be missing.  Otherwise, use a `Jid` enum.
 #[derive(Clone, PartialEq, Eq, Hash)]
 pub struct FullJid {
-    /// The node part of the Jabber ID, if it exists, else None.
+    inner: InnerJid,
-    pub node: Option<String>,
-    /// The domain of the Jabber ID.
-    pub domain: String,
-    /// The resource of the Jabber ID.
-    pub resource: String,
 }
 
 /// A struct representing a bare Jabber ID.
@@ -195,10 +225,7 @@ pub struct FullJid {
 /// there is no case where a resource can be set.  Otherwise, use a `Jid` enum.
 #[derive(Clone, PartialEq, Eq, Hash)]
 pub struct BareJid {
-    /// The node part of the Jabber ID, if it exists, else None.
+    inner: InnerJid,
-    pub node: Option<String>,
-    /// The domain of the Jabber ID.
-    pub domain: String,
 }
 
 impl From<FullJid> for String {
@@ -209,15 +236,7 @@ impl From<FullJid> for String {
 
 impl From<&FullJid> for String {
     fn from(jid: &FullJid) -> String {
-        let mut string = String::new();
+        jid.inner.normalized.clone()
-        if let Some(ref node) = jid.node {
-            string.push_str(node);
-            string.push('@');
-        }
-        string.push_str(&jid.domain);
-        string.push('/');
-        string.push_str(&jid.resource);
-        string
     }
 }
 
@@ -229,22 +248,7 @@ impl From<BareJid> for String {
 
 impl From<&BareJid> for String {
     fn from(jid: &BareJid) -> String {
-        let mut string = String::new();
+        jid.inner.normalized.clone()
-        if let Some(ref node) = jid.node {
-            string.push_str(node);
-            string.push('@');
-        }
-        string.push_str(&jid.domain);
-        string
-    }
-}
-
-impl From<FullJid> for BareJid {
-    fn from(full: FullJid) -> BareJid {
-        BareJid {
-            node: full.node,
-            domain: full.domain,
-        }
     }
 }
 
@@ -292,110 +296,11 @@ impl Serialize for BareJid {
     }
 }
 
-enum ParserState {
-    Node,
-    Domain,
-    Resource,
-}
-
-type StringJid = (Option<String>, String, Option<String>);
-fn _from_str(s: &str) -> Result<StringJid, JidParseError> {
-    // TODO: very naive, may need to do it differently
-    let iter = s.chars();
-    let mut buf = String::with_capacity(s.len());
-    let mut state = ParserState::Node;
-    let mut node = None;
-    let mut domain = None;
-    let mut resource = None;
-    for c in iter {
-        match state {
-            ParserState::Node => {
-                match c {
-                    '@' => {
-                        if buf.is_empty() {
-                            return Err(JidParseError::EmptyNode);
-                        }
-                        state = ParserState::Domain;
-                        node = Some(buf.clone()); // TODO: performance tweaks, do not need to copy it
-                        buf.clear();
-                    }
-                    '/' => {
-                        if buf.is_empty() {
-                            return Err(JidParseError::NoDomain);
-                        }
-                        state = ParserState::Resource;
-                        domain = Some(buf.clone()); // TODO: performance tweaks
-                        buf.clear();
-                    }
-                    c => {
-                        buf.push(c);
-                    }
-                }
-            }
-            ParserState::Domain => {
-                match c {
-                    '/' => {
-                        if buf.is_empty() {
-                            return Err(JidParseError::NoDomain);
-                        }
-                        state = ParserState::Resource;
-                        domain = Some(buf.clone()); // TODO: performance tweaks
-                        buf.clear();
-                    }
-                    c => {
-                        buf.push(c);
-                    }
-                }
-            }
-            ParserState::Resource => {
-                buf.push(c);
-            }
-        }
-    }
-    if !buf.is_empty() {
-        match state {
-            ParserState::Node => {
-                domain = Some(buf);
-            }
-            ParserState::Domain => {
-                domain = Some(buf);
-            }
-            ParserState::Resource => {
-                resource = Some(buf);
-            }
-        }
-    } else if let ParserState::Resource = state {
-        return Err(JidParseError::EmptyResource);
-    }
-    let domain = domain.ok_or(JidParseError::NoDomain)?;
-    let (node, domain, resource) = {
-        let node = if let Some(node) = node {
-            Some(nodeprep(&node)?.into_owned())
-        } else {
-            None
-        };
-        let domain = nameprep(&domain)?.into_owned();
-        let resource = if let Some(resource) = resource {
-            Some(resourceprep(&resource)?.into_owned())
-        } else {
-            None
-        };
-
-        (node, domain, resource)
-    };
-    Ok((node, domain, resource))
-}
-
 impl FromStr for FullJid {
     type Err = JidParseError;
 
     fn from_str(s: &str) -> Result<FullJid, JidParseError> {
-        let (ns, ds, rs): StringJid = _from_str(s)?;
+        FullJid::new(s)
-        Ok(FullJid {
-            node: ns,
-            domain: ds,
-            resource: rs.ok_or(JidParseError::NoResource)?,
-        })
     }
 }
 
@@ -430,102 +335,60 @@ impl FullJid {
     ///
     /// ```
     /// use jid::FullJid;
+    /// # use jid::JidParseError;
     ///
-    /// let jid = FullJid::new("node", "domain", "resource");
+    /// # fn main() -> Result<(), JidParseError> {
+    /// let jid = FullJid::new("node@domain/resource")?;
     ///
-    /// assert_eq!(jid.node, Some("node".to_owned()));
+    /// assert_eq!(jid.node(), Some("node"));
-    /// assert_eq!(jid.domain, "domain".to_owned());
+    /// assert_eq!(jid.domain(), "domain");
-    /// assert_eq!(jid.resource, "resource".to_owned());
+    /// assert_eq!(jid.resource(), "resource");
+    /// # Ok(())
+    /// # }
     /// ```
-    pub fn new<NS, DS, RS>(node: NS, domain: DS, resource: RS) -> FullJid
+    pub fn new(s: &str) -> Result<FullJid, JidParseError> {
-    where
+        let inner = InnerJid::new(s)?;
-        NS: Into<String>,
+        if inner.slash.is_some() {
-        DS: Into<String>,
+            Ok(FullJid { inner })
-        RS: Into<String>,
+        } else {
-    {
+            Err(JidParseError::NoResource)
-        FullJid {
-            node: Some(node.into()),
-            domain: domain.into(),
-            resource: resource.into(),
         }
     }
 
-    /// Constructs a new Jabber ID from an existing one, with the node swapped out with a new one.
+    /// The node part of the Jabber ID, if it exists, else None.
-    ///
+    pub fn node(&self) -> Option<&str> {
-    /// # Examples
+        self.inner.node()
-    ///
-    /// ```
-    /// use jid::FullJid;
-    ///
-    /// let jid = FullJid::new("node", "domain", "resource");
-    ///
-    /// assert_eq!(jid.node, Some("node".to_owned()));
-    ///
-    /// let new_jid = jid.with_node("new_node");
-    ///
-    /// assert_eq!(new_jid.node, Some("new_node".to_owned()));
-    /// ```
-    pub fn with_node<NS>(&self, node: NS) -> FullJid
-    where
-        NS: Into<String>,
-    {
-        FullJid {
-            node: Some(node.into()),
-            domain: self.domain.clone(),
-            resource: self.resource.clone(),
-        }
     }
 
-    /// Constructs a new Jabber ID from an existing one, with the domain swapped out with a new one.
+    /// The domain part of the Jabber ID.
-    ///
+    pub fn domain(&self) -> &str {
-    /// # Examples
+        self.inner.domain()
-    ///
-    /// ```
-    /// use jid::FullJid;
-    ///
-    /// let jid = FullJid::new("node", "domain", "resource");
-    ///
-    /// assert_eq!(jid.domain, "domain".to_owned());
-    ///
-    /// let new_jid = jid.with_domain("new_domain");
-    ///
-    /// assert_eq!(new_jid.domain, "new_domain");
-    /// ```
-    pub fn with_domain<DS>(&self, domain: DS) -> FullJid
-    where
-        DS: Into<String>,
-    {
-        FullJid {
-            node: self.node.clone(),
-            domain: domain.into(),
-            resource: self.resource.clone(),
-        }
     }
 
-    /// Constructs a full Jabber ID from a bare Jabber ID, specifying a `resource`.
+    /// The resource of the Jabber ID.  Since this is a full JID it is always present.
-    ///
+    pub fn resource(&self) -> &str {
-    /// # Examples
+        self.inner.resource().unwrap()
-    ///
+    }
-    /// ```
+
-    /// use jid::FullJid;
+    /// Extract a bare JID from this full JID, throwing away the resource.
-    ///
+    pub fn to_bare(&self) -> BareJid {
-    /// let jid = FullJid::new("node", "domain", "resource");
+        let slash = self.inner.slash.unwrap().get() as usize;
-    ///
+        let normalized = self.inner.normalized[..slash].to_string();
-    /// assert_eq!(jid.resource, "resource".to_owned());
+        let inner = InnerJid {
-    ///
+            normalized,
-    /// let new_jid = jid.with_resource("new_resource");
+            at: self.inner.at,
-    ///
+            slash: None,
-    /// assert_eq!(new_jid.resource, "new_resource");
+        };
-    /// ```
+        BareJid { inner }
-    pub fn with_resource<RS>(&self, resource: RS) -> FullJid
+    }
-    where
+
-        RS: Into<String>,
+    /// Transforms this full JID into a bare JID, throwing away the resource.
-    {
+    pub fn into_bare(mut self) -> BareJid {
-        FullJid {
+        let slash = self.inner.slash.unwrap().get() as usize;
-            node: self.node.clone(),
+        self.inner.normalized.truncate(slash);
-            domain: self.domain.clone(),
+        self.inner.normalized.shrink_to_fit();
-            resource: resource.into(),
+        self.inner.slash = None;
-        }
+        BareJid { inner: self.inner }
     }
 }
 
@@ -533,11 +396,7 @@ impl FromStr for BareJid {
     type Err = JidParseError;
 
     fn from_str(s: &str) -> Result<BareJid, JidParseError> {
-        let (ns, ds, _rs): StringJid = _from_str(s)?;
+        BareJid::new(s)
-        Ok(BareJid {
-            node: ns,
-            domain: ds,
-        })
     }
 }
 
@@ -550,95 +409,33 @@ impl BareJid {
     ///
     /// ```
     /// use jid::BareJid;
+    /// # use jid::JidParseError;
     ///
-    /// let jid = BareJid::new("node", "domain");
+    /// # fn main() -> Result<(), JidParseError> {
+    /// let jid = BareJid::new("node@domain")?;
     ///
-    /// assert_eq!(jid.node, Some("node".to_owned()));
+    /// assert_eq!(jid.node(), Some("node"));
-    /// assert_eq!(jid.domain, "domain".to_owned());
+    /// assert_eq!(jid.domain(), "domain");
+    /// # Ok(())
+    /// # }
     /// ```
-    pub fn new<NS, DS>(node: NS, domain: DS) -> BareJid
+    pub fn new(s: &str) -> Result<BareJid, JidParseError> {
-    where
+        let inner = InnerJid::new(s)?;
-        NS: Into<String>,
+        if inner.slash.is_none() {
-        DS: Into<String>,
+            Ok(BareJid { inner })
-    {
+        } else {
-        BareJid {
+            Err(JidParseError::ResourceInBareJid)
-            node: Some(node.into()),
-            domain: domain.into(),
         }
     }
 
-    /// Constructs a bare Jabber ID containing only a `domain`.
+    /// The node part of the Jabber ID, if it exists, else None.
-    ///
+    pub fn node(&self) -> Option<&str> {
-    /// This is of the form `domain`.
+        self.inner.node()
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// use jid::BareJid;
-    ///
-    /// let jid = BareJid::domain("domain");
-    ///
-    /// assert_eq!(jid.node, None);
-    /// assert_eq!(jid.domain, "domain".to_owned());
-    /// ```
-    pub fn domain<DS>(domain: DS) -> BareJid
-    where
-        DS: Into<String>,
-    {
-        BareJid {
-            node: None,
-            domain: domain.into(),
-        }
     }
 
-    /// Constructs a new Jabber ID from an existing one, with the node swapped out with a new one.
+    /// The domain part of the Jabber ID.
-    ///
+    pub fn domain(&self) -> &str {
-    /// # Examples
+        self.inner.domain()
-    ///
-    /// ```
-    /// use jid::BareJid;
-    ///
-    /// let jid = BareJid::domain("domain");
-    ///
-    /// assert_eq!(jid.node, None);
-    ///
-    /// let new_jid = jid.with_node("node");
-    ///
-    /// assert_eq!(new_jid.node, Some("node".to_owned()));
-    /// ```
-    pub fn with_node<NS>(&self, node: NS) -> BareJid
-    where
-        NS: Into<String>,
-    {
-        BareJid {
-            node: Some(node.into()),
-            domain: self.domain.clone(),
-        }
-    }
-
-    /// Constructs a new Jabber ID from an existing one, with the domain swapped out with a new one.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// use jid::BareJid;
-    ///
-    /// let jid = BareJid::domain("domain");
-    ///
-    /// assert_eq!(jid.domain, "domain");
-    ///
-    /// let new_jid = jid.with_domain("new_domain");
-    ///
-    /// assert_eq!(new_jid.domain, "new_domain");
-    /// ```
-    pub fn with_domain<DS>(&self, domain: DS) -> BareJid
-    where
-        DS: Into<String>,
-    {
-        BareJid {
-            node: self.node.clone(),
-            domain: domain.into(),
-        }
     }
 
     /// Constructs a full Jabber ID from a bare Jabber ID, specifying a `resource`.
@@ -648,22 +445,23 @@ impl BareJid {
     /// ```
     /// use jid::BareJid;
     ///
-    /// let bare = BareJid::new("node", "domain");
+    /// let bare = BareJid::new("node@domain").unwrap();
-    /// let full = bare.with_resource("resource");
+    /// let full = bare.with_resource("resource").unwrap();
     ///
-    /// assert_eq!(full.node, Some("node".to_owned()));
+    /// assert_eq!(full.node(), Some("node"));
-    /// assert_eq!(full.domain, "domain".to_owned());
+    /// assert_eq!(full.domain(), "domain");
-    /// assert_eq!(full.resource, "resource".to_owned());
+    /// assert_eq!(full.resource(), "resource");
     /// ```
-    pub fn with_resource<RS>(self, resource: RS) -> FullJid
+    pub fn with_resource(&self, resource: &str) -> Result<FullJid, JidParseError> {
-    where
+        let resource = resourceprep(resource).map_err(|_| JidParseError::ResourcePrep)?;
-        RS: Into<String>,
+        let slash = NonZeroU16::new(self.inner.normalized.len() as u16);
-    {
+        let normalized = format!("{}/{resource}", self.inner.normalized);
-        FullJid {
+        let inner = InnerJid {
-            node: self.node,
+            normalized,
-            domain: self.domain,
+            at: self.inner.at,
-            resource: resource.into(),
+            slash,
-        }
+        };
+        Ok(FullJid { inner })
     }
 }
 
@@ -717,7 +515,6 @@ mod tests {
     use super::*;
 
     use std::collections::HashMap;
-    use std::str::FromStr;
 
     macro_rules! assert_size (
         ($t:ty, $sz:expr) => (
@@ -728,32 +525,28 @@ mod tests {
     #[cfg(target_pointer_width = "32")]
     #[test]
     fn test_size() {
-        assert_size!(BareJid, 24);
+        assert_size!(BareJid, 16);
-        assert_size!(FullJid, 36);
+        assert_size!(FullJid, 16);
-        assert_size!(Jid, 36);
+        assert_size!(Jid, 20);
     }
 
     #[cfg(target_pointer_width = "64")]
     #[test]
     fn test_size() {
-        assert_size!(BareJid, 48);
+        assert_size!(BareJid, 32);
-        assert_size!(FullJid, 72);
+        assert_size!(FullJid, 32);
-        assert_size!(Jid, 72);
+        assert_size!(Jid, 40);
     }
 
     #[test]
     fn can_parse_full_jids() {
         assert_eq!(
             FullJid::from_str("a@b.c/d"),
-            Ok(FullJid::new("a", "b.c", "d"))
+            Ok(FullJid::new("a@b.c/d").unwrap())
         );
         assert_eq!(
             FullJid::from_str("b.c/d"),
-            Ok(FullJid {
+            Ok(FullJid::new("b.c/d").unwrap())
-                node: None,
-                domain: "b.c".to_owned(),
-                resource: "d".to_owned(),
-            })
         );
 
         assert_eq!(FullJid::from_str("a@b.c"), Err(JidParseError::NoResource));
@@ -762,23 +555,11 @@ mod tests {
 
     #[test]
     fn can_parse_bare_jids() {
-        assert_eq!(BareJid::from_str("a@b.c/d"), Ok(BareJid::new("a", "b.c")));
         assert_eq!(
-            BareJid::from_str("b.c/d"),
+            BareJid::from_str("a@b.c"),
-            Ok(BareJid {
+            Ok(BareJid::new("a@b.c").unwrap())
-                node: None,
-                domain: "b.c".to_owned(),
-            })
-        );
-
-        assert_eq!(BareJid::from_str("a@b.c"), Ok(BareJid::new("a", "b.c")));
-        assert_eq!(
-            BareJid::from_str("b.c"),
-            Ok(BareJid {
-                node: None,
-                domain: "b.c".to_owned(),
-            })
         );
+        assert_eq!(BareJid::from_str("b.c"), Ok(BareJid::new("b.c").unwrap()));
     }
 
     #[test]
@@ -792,55 +573,55 @@ mod tests {
 
     #[test]
     fn full_to_bare_jid() {
-        let bare: BareJid = FullJid::new("a", "b.c", "d").into();
+        let bare: BareJid = FullJid::new("a@b.c/d").unwrap().to_bare();
-        assert_eq!(bare, BareJid::new("a", "b.c"));
+        assert_eq!(bare, BareJid::new("a@b.c").unwrap());
     }
 
     #[test]
     fn bare_to_full_jid() {
         assert_eq!(
-            BareJid::new("a", "b.c").with_resource("d"),
+            BareJid::new("a@b.c").unwrap().with_resource("d").unwrap(),
-            FullJid::new("a", "b.c", "d")
+            FullJid::new("a@b.c/d").unwrap()
         );
     }
 
     #[test]
     fn node_from_jid() {
         assert_eq!(
-            Jid::Full(FullJid::new("a", "b.c", "d")).node(),
+            Jid::Full(FullJid::new("a@b.c/d").unwrap()).node(),
-            Some(String::from("a")),
+            Some("a"),
         );
     }
 
     #[test]
     fn domain_from_jid() {
-        assert_eq!(
+        assert_eq!(Jid::Bare(BareJid::new("a@b.c").unwrap()).domain(), "b.c");
-            Jid::Bare(BareJid::new("a", "b.c")).domain(),
-            String::from("b.c"),
-        );
     }
 
     #[test]
     fn jid_to_full_bare() {
-        let full = FullJid::new("a", "b.c", "d");
+        let full = FullJid::new("a@b.c/d").unwrap();
-        let bare = BareJid::new("a", "b.c");
+        let bare = BareJid::new("a@b.c").unwrap();
 
-        assert_eq!(FullJid::try_from(Jid::Full(full.clone())), Ok(full.clone()),);
+        assert_eq!(FullJid::try_from(Jid::Full(full.clone())), Ok(full.clone()));
         assert_eq!(
             FullJid::try_from(Jid::Bare(bare.clone())),
             Err(JidParseError::NoResource),
         );
-        assert_eq!(BareJid::from(Jid::Full(full.clone())), bare.clone(),);
+        assert_eq!(Jid::Bare(full.clone().to_bare()), bare.clone());
-        assert_eq!(BareJid::from(Jid::Bare(bare.clone())), bare,);
+        assert_eq!(Jid::Bare(bare.clone()), bare);
     }
 
     #[test]
     fn serialise() {
         assert_eq!(
-            String::from(FullJid::new("a", "b", "c")),
+            String::from(FullJid::new("a@b/c").unwrap()),
             String::from("a@b/c")
         );
-        assert_eq!(String::from(BareJid::new("a", "b")), String::from("a@b"));
+        assert_eq!(
+            String::from(BareJid::new("a@b").unwrap()),
+            String::from("a@b")
+        );
     }
 
     #[test]
@@ -867,16 +648,19 @@ mod tests {
     #[test]
     fn display_jids() {
         assert_eq!(
-            format!("{}", FullJid::new("a", "b", "c")),
+            format!("{}", FullJid::new("a@b/c").unwrap()),
-            String::from("a@b/c")
-        );
-        assert_eq!(format!("{}", BareJid::new("a", "b")), String::from("a@b"));
-        assert_eq!(
-            format!("{}", Jid::Full(FullJid::new("a", "b", "c"))),
             String::from("a@b/c")
         );
         assert_eq!(
-            format!("{}", Jid::Bare(BareJid::new("a", "b"))),
+            format!("{}", BareJid::new("a@b").unwrap()),
+            String::from("a@b")
+        );
+        assert_eq!(
+            format!("{}", Jid::Full(FullJid::new("a@b/c").unwrap())),
+            String::from("a@b/c")
+        );
+        assert_eq!(
+            format!("{}", Jid::Bare(BareJid::new("a@b").unwrap())),
             String::from("a@b")
         );
     }
@@ -886,31 +670,31 @@ mod tests {
     fn minidom() {
         let elem: minidom::Element = "<message xmlns='ns1' from='a@b/c'/>".parse().unwrap();
         let to: Jid = elem.attr("from").unwrap().parse().unwrap();
-        assert_eq!(to, Jid::Full(FullJid::new("a", "b", "c")));
+        assert_eq!(to, Jid::Full(FullJid::new("a@b/c").unwrap()));
 
         let elem: minidom::Element = "<message xmlns='ns1' from='a@b'/>".parse().unwrap();
         let to: Jid = elem.attr("from").unwrap().parse().unwrap();
-        assert_eq!(to, Jid::Bare(BareJid::new("a", "b")));
+        assert_eq!(to, Jid::Bare(BareJid::new("a@b").unwrap()));
 
         let elem: minidom::Element = "<message xmlns='ns1' from='a@b/c'/>".parse().unwrap();
         let to: FullJid = elem.attr("from").unwrap().parse().unwrap();
-        assert_eq!(to, FullJid::new("a", "b", "c"));
+        assert_eq!(to, FullJid::new("a@b/c").unwrap());
 
         let elem: minidom::Element = "<message xmlns='ns1' from='a@b'/>".parse().unwrap();
         let to: BareJid = elem.attr("from").unwrap().parse().unwrap();
-        assert_eq!(to, BareJid::new("a", "b"));
+        assert_eq!(to, BareJid::new("a@b").unwrap());
     }
 
     #[cfg(feature = "minidom")]
     #[test]
     fn minidom_into_attr() {
-        let full = FullJid::new("a", "b", "c");
+        let full = FullJid::new("a@b/c").unwrap();
         let elem = minidom::Element::builder("message", "jabber:client")
             .attr("from", full.clone())
             .build();
         assert_eq!(elem.attr("from"), Some(String::from(full).as_ref()));
 
-        let bare = BareJid::new("a", "b");
+        let bare = BareJid::new("a@b").unwrap();
         let elem = minidom::Element::builder("message", "jabber:client")
             .attr("from", bare.clone())
             .build();
@@ -926,7 +710,7 @@ mod tests {
     #[test]
     fn stringprep() {
         let full = FullJid::from_str("Test@☃.coM/Test™").unwrap();
-        let equiv = FullJid::new("test", "☃.com", "TestTM");
+        let equiv = FullJid::new("test@☃.com/TestTM").unwrap();
         assert_eq!(full, equiv);
     }
 }