[openstack-dev] [nova][oslo.utils] Bug-1680130 Check validation of UUID length

Sean Dague sean at dague.net
Wed Apr 26 14:55:14 UTC 2017


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

-- 
Sean Dague
http://dague.net



More information about the OpenStack-dev mailing list