Open Stack

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:doug at 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-request at 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.