Display implementation for Uuid::from_bytes / Parse::InvalidLength is incorrect #281

New issue

Closed

opened 2018-07-18 18:11:08 -05:00 by DarrenTsung · 2 comments

DarrenTsung commented

2018-07-18 18:11:08 -05:00

(Migrated from github.com)

Describe the bug
When trying to parse Uuid using Uuid::from_bytes, an invalid byte array of size 32 or 36 gives you an incorrect Display string:

Invalid length; expecting 32 or 36 chars, found 36

This is because the code is checking for length 16: https://docs.rs/uuid/0.6.5/src/uuid/lib.rs.html#711-720, and the error message is outputting adapter:: UUID_HYPHENATED_LENGTH and adapter:: UUID_SIMPLE_LENGTH which are 36 and 32: https://github.com/uuid-rs/uuid/blob/master/src/adapter.rs#L19.

To Reproduce
Steps to reproduce the behavior:

Create a byte array of size 32 or 36.
Try to parse it with Uuid::from_bytes.

Expected behavior
A correct error message should appear, not sure what the ideal error message for this is.

Screenshots
N/A

Specifications (please complete the following information):

Target: stable-x86_64-apple-darwin
Version: 0.6.5
Features Enabled: None

Additional context
N/A

Other
N/A

**Describe the bug** When trying to parse Uuid using `Uuid::from_bytes`, an invalid byte array of size 32 or 36 gives you an incorrect Display string: `Invalid length; expecting 32 or 36 chars, found 36` This is because the code is checking for length 16: https://docs.rs/uuid/0.6.5/src/uuid/lib.rs.html#711-720, and the error message is outputting `adapter:: UUID_HYPHENATED_LENGTH` and `adapter:: UUID_SIMPLE_LENGTH` which are 36 and 32: https://github.com/uuid-rs/uuid/blob/master/src/adapter.rs#L19. **To Reproduce** Steps to reproduce the behavior: 1. Create a byte array of size 32 or 36. 2. Try to parse it with `Uuid::from_bytes`. **Expected behavior** A correct error message should appear, not sure what the ideal error message for this is. **Screenshots** N/A **Specifications (please complete the following information):** - Target: stable-x86_64-apple-darwin - Version: 0.6.5 - Features Enabled: None **Additional context** N/A **Other** N/A

Dylan-DPC-zz commented

2018-07-19 06:57:31 -05:00

(Migrated from github.com)

hi @DarrenTsung thanks for reporting this.

kinggoesgaming commented

2018-07-21 15:33:32 -05:00

(Migrated from github.com)

I think the main issue I see here is that ParseError is primarily designed around parsing strings. However, our overall design expected bytes.

The way I think we can fix this is by leaving the current error message as is. I cannot change the message without making the error confusing for people using string parsing.

Instead in 0.7.0(which already has a refactor for the error type planned, see #276), I can have 2 invalid length variants, one for string and one for bytes.

I think the main issue I see here is that `ParseError` is primarily designed around parsing strings. However, our overall design expected bytes. The way I think we can fix this is by leaving the current error message as is. I cannot change the message without making the error confusing for people using string parsing. Instead in `0.7.0`(which already has a refactor for the error type planned, see #276), I can have 2 invalid length variants, one for string and one for bytes.