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

uuid constructor accept invalid strings (extra dash) #80938

Open
CedricCabessa mannequin opened this issue Apr 30, 2019 · 5 comments
Open

uuid constructor accept invalid strings (extra dash) #80938

CedricCabessa mannequin opened this issue Apr 30, 2019 · 5 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@CedricCabessa
Copy link
Mannequin

CedricCabessa mannequin commented Apr 30, 2019

BPO 36757
Nosy @taleinat, @MojoVampire, @Windsooon, @CedricCabessa

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-04-30.09:19:48.878>
labels = ['3.7', '3.8', 'type-bug', 'library', '3.9']
title = 'uuid constructor accept invalid strings (extra dash)'
updated_at = <Date 2019-08-05.10:21:04.013>
user = 'https://github.com/CedricCabessa'

bugs.python.org fields:

activity = <Date 2019-08-05.10:21:04.013>
actor = 'taleinat'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2019-04-30.09:19:48.878>
creator = 'C\xc3\xa9dric Cabessa'
dependencies = []
files = []
hgrepos = []
issue_num = 36757
keywords = []
message_count = 5.0
messages = ['341141', '341154', '341216', '341238', '349045']
nosy_count = 4.0
nosy_names = ['taleinat', 'josh.r', 'Windson Yang', 'C\xc3\xa9dric Cabessa']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36757'
versions = ['Python 2.7', 'Python 3.7', 'Python 3.8', 'Python 3.9']

@CedricCabessa
Copy link
Mannequin Author

CedricCabessa mannequin commented Apr 30, 2019

UUID constructor accept string with too many dashes or keyword like urn: / uuid:

For eg, this code do not raise

>>> import uuid
>>> uuid.UUID('0be--468urn:urn:urn:urn:54-4bf9-41----------d4-9697-41d735uuid:4fbe85uuid:')
UUID('0be46854-4bf9-41d4-9697-41d7354fbe85')

For the context, we use a validator based on uuid.UUID for an API.
Some customer send string with a UUID followed by extra -, the validator let it pass but the sql connector raise an exception

We workaround this in our validator, but UUID constructor should not accept string like the one in exemple

@CedricCabessa CedricCabessa mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir labels Apr 30, 2019
@MojoVampire
Copy link
Mannequin

MojoVampire mannequin commented Apr 30, 2019

The documentation does describe a fairly flexible parser. Perhaps it's a little too flexible on stuff like URN prefixes, but I don't think we could start enforcing a stricter class of hyphen separations without potentially breaking existing code.

Is there are reason your validator doesn't use uuid.UUID to normalize the value? That is, whatever the customer provides, why not use the result of stringifying the resulting UUID, rather than just convert to UUID to validate, then throwing it away? As long as the result is compatible with your sql connector, and logically equivalent to what the customer provided, that seems a valid solution.

@CedricCabessa
Copy link
Mannequin Author

CedricCabessa mannequin commented May 1, 2019

Is there are reason your validator doesn't use uuid.UUID to normalize the value? That is, whatever the customer provides, why not use the result of stringifying the resulting UUID

Yes, this is exactly what we do now

However this behaviour is a bit surprising, we were thinking that uuid.UUID could be used as a validator

I understand the risk of breaking existing code with a fix that enforce a strict form.

Maybe a line should be added in the documentation to prevent people using this as a validator without more check?

But ok, you can close the bug if you prefer ... I think there no perfect solution :-)

@Windsooon
Copy link
Mannequin

Windsooon mannequin commented May 2, 2019

Maybe a line should be added in the documentation to prevent people using this as a validator without more check?

I don't expect uuid.UUID could be used as a validator myself, but I agreed we can warn users in the documentation if lots of them confuse about it.

@taleinat
Copy link
Contributor

taleinat commented Aug 5, 2019

I too find this surprising, especially given how thoroughly UUID validates inputs of types other than "hex".

The documentation simply states that for hex input, hypens, curly braces and a URN prefix are optional. In practice, though, it is much more lenient than that, as described here.

Since the UUID has no expert listed, we'll have to decide whether to make the input validation stricter and break backwards-compatibility, or simply make the docs clearer. Clarifying the docs certainly seems simpler, safer and more user-friendly. It also seems reasonable, given that this issue apparently hasn't affected many users.

@taleinat taleinat added the type-bug An unexpected behavior, bug, or error label Aug 5, 2019
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant