erikjara / rut-lib

A Rust library for parsing, format and validate a Chilean ID (RUT)
MIT License
4 stars 3 forks source link

panics if Rut starts with 00 #8

Open jfgodoy opened 3 years ago

jfgodoy commented 3 years ago

Hola Erik! Primero que todo, muchas gracias por el tiempo dedicado a esta librería.

Detecté, que la librería tira un panic si el Rut parte con 00:

use rut_lib::Rut;

fn main() {
    let stringifier_rut = "00951585-7";

    match Rut::from(stringifier_rut) {
        Ok(rut) => {
            println!("RUT: {:#}", rut)
        }
        Err(error) => println!("Error: {:#}", error),
    }
}
erikjara commented 3 years ago

Hola, @jfgodoy Te agradezco muchísimo por levantar el issue, lamentablemente por el contexto que estamos teniendo no he revisado el proyecto y dudo tenerlo en el corto tiempo.

Entiendo que esto es solucionable con un Regex y/o String.replace de los primeros ceros que estén dentro de la entrada recibida, te agradecería a mil si pudieras enviar el PR, con gusto hago la revisión como merge de los cambios.

Muchos ánimos en estos tiempos complejos, y nuevamente gracias por esto.

EstebanBorai commented 3 years ago

Hi @erikjara and @jfgodoy! As #12 is already merged, I guess this should be closed, right?

jfgodoy commented 3 years ago

yes, I'm agree :+1:

erikjara commented 3 years ago

@EstebanBorai @jfgodoy Thanks for the help in this issue! ❤

erikjara commented 3 years ago
Original Text (Spanish)

@jfgodoy Disculpa las molestias, pero me estaba dando cuenta que admitir ceros en la entrada me parecía incorrecto, ya que la función de la librería es decirte si el RUT ingresado es uno de los formatos válidos, y si no, mostrar que es incorrecto.

Para este caso en particular, lo que voy a hacer es lo siguiente: Primero, sí puedes mandar el PR nuevamente a la rama https://github.com/erikjara/rut-lib/tree/fix/parse-input-with-zeroes te lo agradecería muchísimo, te dejo el enlace [aquí](https://github.com/erikjara/rut-lib/compare/fix/parse-input-with-zeroes...jfgodoy:fix/support-for-rut-with-preceding-zeroes?expand=1) para que puedas crearlo rápidamente. Después del merge (en modo rebase), haré un `Negative Lookahead` a la regla de cero que añadiste, y así realmente se arregla el problema que es evitar el `panic` de la librería con la entrada que señalaste en este problema.

@jfgodoy Sorry for the inconvenience, but I was realizing that admitting zeros in the input seemed wrong to me, since the function of the library is to tell you if the RUT input is one of the valid formats, and if not, show that it is wrong.

For this particular case, what I'm going to do is the following steps: First, if you can send the PR again to the branch https://github.com/erikjara/rut-lib/tree/fix/parse-input-with-zeroes I would really appreciate it, I leave the link here, so you can create it quickly.

After the merge (rebase mode), I will do a Negative Lookahead to the zero rule that you added, and this really fixes the problem that is avoiding the panic with the input you pointed out in this problem.

EstebanBorai commented 3 years ago
Original Text (Spanish)

@jfgodoy Disculpa las molestias, pero me estaba dando cuenta que admitir ceros en la entrada me parecía incorrecto, ya que la función de la librería es decirte si el RUT ingresado es uno de los formatos válidos, y si no, mostrar que es incorrecto.

Para este caso en particular, lo que voy a hacer es lo siguiente: Primero, sí puedes mandar el PR nuevamente a la rama https://github.com/erikjara/rut-lib/tree/fix/parse-input-with-zeroes te lo agradecería muchísimo, te dejo el enlace [aquí](https://github.com/erikjara/rut-lib/compare/fix/parse-input-with-zeroes...jfgodoy:fix/support-for-rut-with-preceding-zeroes?expand=1) para que puedas crearlo rápidamente. Después del merge (en modo rebase), haré un `Negative Lookahead` a la regla de cero que añadiste, y así realmente se arregla el problema que es evitar el `panic` de la librería con la entrada que señalaste en este problema.

@jfgodoy Sorry for the inconvenience, but I was realizing that admitting zeros in the input seemed wrong to me, since the function of the library is to tell you if the RUT input is one of the valid formats, and if not, show that it is wrong.

For this particular case, what I'm going to do is the following steps:

First, if you can send the PR again to the branch https://github.com/erikjara/rut-lib/tree/fix/parse-input-with-zeroes I would really appreciate it, I leave the link here, so you can create it quickly.

After the merge (rebase mode), I will do a Negative Lookahead to the zero rule that you added, and this really fixes the problem that is avoiding the panic with the input you pointed out in this problem.

I'm agree with you, its kind of misleading to have a RUT being digested by the library and acting unsound if the RUT doesn't have the right format.

Thinking out loud...

I think the best approach is the one we have today, but we could also provide a "sanitize" function that attempts to fix the format and then digest it as we are doing now.

This way we don't break the current version by changing the internals of the API.

I would expect RUT numbers with preceding zeroes to return a Error::InvalidFormat error and have a Rut::sanitize that receives arbitrary RUT numbers, with preceding zeroes for instance, and retrieve them formatted.

This is always fine as long as you don't mess up with the RUT per se because we could mutate the real input by attempting to sanitize it.

Those are my 2¢ on this issue, but I think it would be great to have a description of what do you expect (speaking behavior) from this crate in such scenarios?