Open Stack

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