classification
Title: uuid returns version more than 5
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: barry, berker.peksag, cheryl.sabella, conqp, jophine pranjal, taleinat, twouters, xtreak
Priority: normal Keywords: patch

Created on 2018-09-19 06:33 by jophine pranjal, last changed 2020-01-12 13:57 by taleinat.

Pull Requests
URL Status Linked Edit
PR 9413 open python-dev, 2018-09-19 09:13
PR 9417 open python-dev, 2018-09-19 12:04
Messages (12)
msg325707 - (view) Author: jophine antony (jophine pranjal) * Date: 2018-09-19 06:33
When I validate the invalid uuid (RFC 4122 variant) for example '481a8de5-f0d1-f211-b425-e41f134196da'. It returns version number as 15 which is invalid. We need to retrun None in this case as we do for invalid UUIDs  based on RFC 4122.

------snip snip-------
from uuid import UUID
test_uuid_str = '481a8de5-f0d1-f211-b425-e41f134196da'
print("UUID version: {}".format(UUID(test_uuid_str).version))
print("UUID variant: {}".format(UUID(test_uuid_str).variant))
------snip snip-------

------Result----------
UUID version: 15
UUID variant: specified in RFC 4122
------Result----------
msg325741 - (view) Author: Richard Neumann (conqp) * Date: 2018-09-19 10:27
I'm not sure whether the property method should be changed.
I think it'd be more appropriate to raise a value error upon __init__ in this case as it is done with other checks.
msg325749 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018-09-19 12:06
I think this is a valid UUID and trying this on JDK 9 also returns 15 for the version like Python. Am I missing something here?

➜  ~ jshell
|  Welcome to JShell -- Version 9.0.4
|  For an introduction type: /help intro

jshell> import java.util.*

jshell> UUID.fromString("481a8de5-f0d1-f211-b425-e41f134196da").version()
$2 ==> 15

jshell> UUID.fromString("481a8de5-f0d1-f211-b425-e41f134196da").variant()
$3 ==> 2

JDK implementation : http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/java/util/UUID.java#l247


Thanks
msg325750 - (view) Author: Richard Neumann (conqp) * Date: 2018-09-19 12:11
@xtreak RFC 4122, section 4.1.3. specifies only versions 1 to 5.
For explicitely checking the version, there is already a test in UUID.__init__, raising a ValueError on not 1<= verision 1<=5.
I moved it to the bottom of __init__, i.e. after setting the "int" property, causing the test to run on the actual instance's property value.
msg325751 - (view) Author: Richard Neumann (conqp) * Date: 2018-09-19 12:18
Typos:
"For explicitely checking the version" → "For explicitely *setting* the version".
"on not 1<= verision 1<=5" → "on not 1 <= version <= 5".
msg325752 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018-09-19 12:35
Seems there is an open issue about this : https://bugs.python.org/issue31958

Thanks
msg325810 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-09-19 22:39
Thanks for the report and for the PRs. The approach in PR 9417 seems reasonable to me, but we need to add some tests for all supported platforms and fix the test_windll_getnode test.

I'm removing Python 3.6 from the versions list since it's nearly end of its bugfix maintenance period and fixing this relatively harmless bug may break people's code in the wild.
msg325846 - (view) Author: jophine antony (jophine pranjal) * Date: 2018-09-20 07:31
I had raised a PR in github when i have created this ticket but forgot to add it here: https://github.com/python/cpython/pull/9413
msg325907 - (view) Author: Richard Neumann (conqp) * Date: 2018-09-20 17:10
I updated my pull request.
Since "_windll_getnode()" is only returning a (random?) node for a UUID, I circumevented the value checking by introducing a new keyword-only argument "strict" defaulting to "True", there being set to "False".
msg325915 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2018-09-20 17:33
> I had raised a PR in github when i have created this ticket but forgot
> to add it here: https://github.com/python/cpython/pull/9413

Thanks for the PR, Jophine :) I saw your PR yesterday, but I thought fixing this in __init__ would be a better solution. I may be wrong, though, so let's see what other core devs think.
msg359829 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2020-01-12 02:40
Updating version and adding some people to the nosy list to review the two suggested pull requests for this issue.  Thanks!
msg359848 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2020-01-12 13:57
The uuid module is likely used on a huge variety of operating systems and hardware; I feel making UUID.__init__() reject values which it accepted until now would unnecessarily break existing code.  While raising an exception in __init__ seems more natural and correct in theory, it is also a larger break in backwards-compatibility.

I'm specifically worried by the _windll() example in PR GH-9417, in which a non-RFC 4122 conforming UUID is created, leading to the addition of the new "strict" keyword argument so that we can disable the new check in __init__ in this case.  There's a good chance that there are other such scenarios of which we're simply not yet aware.

In light of this, returning None for UUID.version, as in PR GH-9413, actually seems like a very reasonable solution.
History
Date User Action Args
2020-01-12 13:57:11taleinatsetmessages: + msg359848
2020-01-12 13:33:13taleinatsetversions: + Python 3.7
2020-01-12 02:40:48cheryl.sabellasetnosy: + cheryl.sabella, twouters, taleinat, barry

messages: + msg359829
versions: + Python 3.9, - Python 3.7
2018-09-20 17:33:39berker.peksagsetmessages: + msg325915
2018-09-20 17:10:45conqpsetmessages: + msg325907
2018-09-20 07:31:08jophine pranjalsetmessages: + msg325846
2018-09-19 22:39:22berker.peksagsetversions: - Python 3.6
nosy: + berker.peksag

messages: + msg325810

components: + Library (Lib)
type: behavior
2018-09-19 22:33:43berker.peksaglinkissue31958 superseder
2018-09-19 12:35:50xtreaksetmessages: + msg325752
2018-09-19 12:18:05conqpsetmessages: + msg325751
2018-09-19 12:11:50conqpsetmessages: + msg325750
2018-09-19 12:06:59xtreaksetmessages: + msg325749
2018-09-19 12:04:28python-devsetpull_requests: + pull_request8837
2018-09-19 10:27:53conqpsetnosy: + conqp
messages: + msg325741
2018-09-19 10:15:18xtreaksetnosy: + xtreak
2018-09-19 09:13:51python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request8833
2018-09-19 06:33:06jophine pranjalcreate