Hi All, As per suggested by Doug, this is what we will implement.
Add new method (format_canonical_uuid()) in oslo_utils.uuidutils module which will return valid uuid and then all other projects which are using is_uuid_like() method needs to call this method to get valid uuid. This approach sounds reasonable to me. Please let me know your opinion about this approach. Thank you, Abhishek Kekane -----Original Message----- From: Doug Hellmann [mailto:d...@doughellmann.com] Sent: Wednesday, April 26, 2017 9:01 PM To: openstack-dev Subject: Re: [openstack-dev] [nova][oslo.utils] Bug-1680130 Check validation of UUID length Excerpts from Sean Dague's message of 2017-04-26 10:55:14 -0400: > On 04/26/2017 10:47 AM, Doug Hellmann wrote: > > Excerpts from Sean Dague's message of 2017-04-26 09:01:32 -0400: > >> On 04/26/2017 08:36 AM, Doug Hellmann wrote: > >>> Excerpts from Kekane, Abhishek's message of 2017-04-26 07:00:22 +0000: > >>>> Hi All, > >>>> > >>>> As per suggested by @jay_pipes's > >>>> if val.count('-') not in (0, 4): > >>>> raise TypeError > >>>> > >>>> It is not sufficient solution because "is_uuid_like" returns only True > >>>> or False. > >>>> For example, > >>>> > >>>> If user passes uuid like "urn:11111111-2222-4444-5555-666666666666" or > >>>> "urn:uuid:11111111-2222-4444-5555-666666666666" then "is_uuid_like" > >>>> method returns True as it is valid uuid format, but when this uuid tries > >>>> to insert into database table then it gives DBDataError because the > >>>> reason is in database "block_device_mapping" table has "volume_id" field > >>>> of 36 characters only so while inserting data to the table through > >>>> 'BlockDeviceMapping' object it raises DBDataError. > >>>> > >>>> Doug's solution of adding another method format_canonical_uuid() which > >>>> would format it with the proper number of hyphens and return actual UUID > >>>> will break backward compatibility IMO. Because of adding this new method > >>>> in oslo_utils then we have to make changes in all projects which are > >>>> using this is_uuid_like(). > >>> > >>> I don't understand why adding a new function breaks backwards > >>> compatibility. Can you elaborate on why you think so? > >> > >> I'm not sure why it's believed it would break compatibility, > >> however > >> format_canonical_uuid() isn't what Nova needs here. > >> > >> Nova actually wants to stop bad UUIDs ever getting past our API > >> layer, and just spin back to the user that they handed us corrupt > >> data. Because it will matter later if they try to use things in > >> comparisons. Papering over bad format isn't what we want or intended. > >> > >> I think we will end up needing a "is_uuid" which accepts the > >> standard dashed format only. > >> > >> -Sean > >> > > > > Sure, that's definitely another option, and again a new function > > would be the way to do it and maintain backwards compatibility. > > > > It sounds like there's a chance there's already bad data in the > > database, though? For example a UUID presented without the dashes > > would have passed the existing check and been able to be stored in > > the field because it's shorter than the max length. What happens to > > those records? > > That is a good question, and one where we have to figure out what the > cost of updating that data would be. I do wonder in what operations > that round trips and becomes a good value later. > > But, at a minimum, we want to prevent new bad data from landing. > > -Sean > Maybe preventing writes with bad data, but allowing queries with the existing looser constraint, solves the problem? Presumably users querying against this field already have to enter the UUID in exactly the same way it was recorded, since it's not being converted to a canonical form? Or maybe this is not a field used in queries? Either way, I agree the bad data should be blocked with more strict checks on input. Doug __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ______________________________________________________________________ Disclaimer: This email and any attachments are sent in strictest confidence for the sole use of the addressee and may contain legally privileged, confidential, and proprietary data. If you are not the intended recipient, please advise the sender by replying promptly to this email and then delete and destroy this email and any attachments without any further use, copying or forwarding. __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
