Skip to content

Some API should be unsafe #153

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

Open
max-ym opened this issue Apr 9, 2019 · 2 comments
Open

Some API should be unsafe #153

max-ym opened this issue Apr 9, 2019 · 2 comments

Comments

@max-ym
Copy link

max-ym commented Apr 9, 2019

Function borrowed_name (in quick_xml::events::BytesStart) as is stated in documentation - can be used to create BytesStart from a given name. But if you pass (by mistake or intentionally) attribute data in the string alongside with the name this function does not parse those attributes nor verify if actually passed data is a valid name. It will treat any kind of data as a name even if it is not valid! This can lead to hard to find bugs as it is possible to confuse fn borrowed_name with fn borrowed which seem to do similar things but lead to different internal state of BytesStart.

As this function accepts invalid data it should be marked unsafe. The documentation must be more specific about how to use this function and how not. For example, it must be stated that this function does not verify passed data and if user intents to pass attributes too he/she should use fn borrowed.

@dralley
Copy link
Collaborator

dralley commented Jul 29, 2022

borrowed_name() doesn't exist anymore (in the current unreleased master branch)

@dralley dralley closed this as completed Jul 29, 2022
@Mingun
Copy link
Collaborator

Mingun commented Jul 30, 2022

I don't think that this is done. Yes, the borrowed_name identifier does not exist anymore, but we still have a functions, that should be unsafe in order to not break invariants, like

quick-xml/src/events/mod.rs

Lines 158 to 171 in ad57bc2

/// Creates a new `BytesStart` from the given content (name + attributes).
///
/// # Warning
///
/// `&content[..name_len]` must be a valid name, and the remainder of `content`
/// must be correctly-formed attributes. Neither are checked, it is possible
/// to generate invalid XML if `content` or `name_len` are incorrect.
#[inline]
pub fn from_content<C: Into<Cow<'a, str>>>(content: C, name_len: usize) -> Self {
BytesStart {
buf: str_cow_to_bytes(content),
name_len,
}
}

@Mingun Mingun reopened this Jul 30, 2022
@Mingun Mingun changed the title borrowed_name should be unsafe Some API should be unsafe Jul 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants