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

Octal numbers are parsed according to 1.2 spec not 1.1 #17

Open
firstdorsal opened this issue Feb 10, 2025 · 1 comment
Open

Octal numbers are parsed according to 1.2 spec not 1.1 #17

firstdorsal opened this issue Feb 10, 2025 · 1 comment

Comments

@firstdorsal
Copy link

In yaml 1.1 this is valid octal:

defaultMode: 0400

In yaml 1.2 only this is valid

defaultMode: 0o400

serde-yaml and serde-yaml-ng parse it according to 1.2 spec

This a problem when for example parsing kubernetes manifests because kubernetes uses 1.1

Adding this to parse_unsigned_int should implement the old behavior, allowing both:

fn parse_unsigned_int<T>(
    scalar: &str,
    from_str_radix: fn(&str, radix: u32) -> Result<T, ParseIntError>,
) -> Option<T> {
    let unpositive = scalar.strip_prefix('+').unwrap_or(scalar);
    if let Some(rest) = unpositive.strip_prefix("0x") {
        if rest.starts_with(['+', '-']) {
            return None;
        }
        if let Ok(int) = from_str_radix(rest, 16) {
            return Some(int);
        }
    }
    if let Some(rest) = unpositive.strip_prefix("0o") {
        if rest.starts_with(['+', '-']) {
            return None;
        }
        if let Ok(int) = from_str_radix(rest, 8) {
            return Some(int);
        }
    }
    if let Some(rest) = unpositive.strip_prefix("0b") {
        if rest.starts_with(['+', '-']) {
            return None;
        }
        if let Ok(int) = from_str_radix(rest, 2) {
            return Some(int);
        }
    }



    // THIS
    if unpositive.starts_with("0")
        && unpositive.len() > 1
        && unpositive.chars().all(|c| c.is_digit(8))
    {
        if let Ok(int) = from_str_radix(unpositive, 8) {
            return Some(int);
        }
    }




    if unpositive.starts_with(['+', '-']) {
        return None;
    }
    if digits_but_not_number(scalar) {
        return None;
    }
    from_str_radix(unpositive, 10).ok()
}
@acatton acatton added bug Something isn't working and removed bug Something isn't working labels Feb 10, 2025
@acatton
Copy link
Owner

acatton commented Feb 10, 2025

Yeah it's unclear to me if the documentation is wrong, or if this is a bug. I think the documentation is wrong.

Following #2, I specified in the README that this crate only supports YAML 1.1.

However, we have plan in #3 to support YAML 1.2.

This is difficult, because this bug comes from unsafe-libyaml which is an old transpiled version of the C library "libyaml" which has many bugs like this. We could upgrade, but I'm in favor of migrating to libyaml-safer (see #5)

In the mean time this issue is a real issue, that I will leave open. But it might get closed with "won't fix, we are now supporting 1.2.", since #5 is the task I want to tackle next, in order to fix #3.

I'm leaving this issue open for documentation purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants