11:11 a.m.
On 05/09/2013 09:58 AM, Martin Kletzander wrote:
On 05/09/2013 01:43 PM, John Ferlan wrote:
> On 05/09/2013 06:59 AM, Martin Kletzander wrote:
>> On 04/30/2013 08:19 PM, John Ferlan wrote:
>>> ---
>>> src/conf/domain_conf.c | 24 ++++++++++++++++++++++++
>>> 1 file changed, 24 insertions(+)
I guess I agree in principal that a month of 99 or a date of 99 would be
incorrect and since we're doing some sort of validation it wouldn't hurt
to do a bit more. Doing full blown is the days in the month right and
handling leap year - is just outside the realm. My guess is that
somewhere some code will do a similar strptime() like call anyway.
So I made the change:
*ptr != '/' ||
virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_year) < 0 ||
*ptr != '\0' ||
+ (tm.tm_mon < 0 || tm.tm_mon > 12) ||
+ (tm.tm_mday < 0 || tm.tm_mday > 31) ||
(tm.tm_year < 0 || (tm.tm_year >= 100 && tm.tm_year <
1900))) {
- virReportError(VIR_ERR_XML_DETAIL,
- _("Invalid BIOS 'date' format: %s"),
- def->sysinfo->bios_date);
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("Invalid BIOS 'date' format"));
>>
>>> diff --git a/src/conf/domain_conf.c
b/src/conf/domain_conf.c
>>> index a8b5dfd..43273f8 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -11591,6 +11591,30 @@ virDomainDefParseXML(xmlDocPtr xml,
>>> goto error;
>>> }
>>> }
>>> + if (def->sysinfo->bios_date != NULL) {
>>> + char *date = def->sysinfo->bios_date;
>>> + char *ptr;
>>> + struct tm tm;
>>> + memset(&tm, 0, sizeof(tm));
>>> +
>>> + /* Validate just the format of the date
>>> + * Expect mm/dd/yyyy or mm/dd/yy,
>>> + * where yy must be 00->99 and would be assumed to be 19xx
>>> + * a yyyy date should be 1900 and beyond
>>> + */
>>> + if (virStrToLong_i(date, &ptr, 10, &tm.tm_mon) < 0
||
>>> + *ptr != '/' ||
>>> + virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_mday) < 0
||
>>> + *ptr != '/' ||
>>> + virStrToLong_i(ptr+1, &ptr, 10, &tm.tm_year) < 0
||
>>> + *ptr != '\0' ||
>>> + (tm.tm_year < 0 || (tm.tm_year >= 100 &&
tm.tm_year < 1900))) {
>>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>>
>> Seems like another abuse of internal error, but I don't know what to use
here,
>> properly. Maybe VIR_ERR_XML_DETAIL?
>>
>>> + _("Invalid BIOS 'date' format:
%s"),
>>> + def->sysinfo->bios_date);
>>
>> Unnecessarily long, you can do 's/def->sysinfo->bios_//' and save
one
>> line here ;-)
>>
>>> + goto error;
>>> + }
>>> + }
>>> }
>>>
>>> if ((tmp = virXPathString("string(./os/smbios/@mode)", ctxt)))
{
>>
>
> FYI: The above is essentially a cut-n-reformat for this particular need
> of virDomainGraphicsAuthDefParseXML(). And while I agree it's an eye
> strain to read - I also tried various strptime() formats then using
> strftime() to format it back..
>
I haven't seen it being used somewhere else, but makes sense also due
to the rest of the mail.
>>
...
And did squash/add the test provided - thanks! I also tried a couple of
other dates (both good and bad) during self testing to make sure the
code validated properly...
Going to do something similar with uuid validation shortly...
John
I'd add it to qemuxml2argvtest with invalid date and
DO_TEST_PARSE_ERROR.
Example (feel free to use it, it's tested):
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios-date.xml
b/tests/qemuxml2argvdata/qemuxml2argv-smbios-date.xml
new file mode 100644
index 0000000..7b2f33a
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios-date.xml
@@ -0,0 +1,23 @@
+<domain type='qemu'>
+ <name>smbios</name>
+ <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <smbios mode="sysinfo"/>
+ </os>
+ <sysinfo type="smbios">
+ <bios>
+ <entry name="date">999/999/123</entry>
+ </bios>
+ </sysinfo>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 1286273..7d5c3d0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -814,6 +814,7 @@ mymain(void)
DO_TEST("smbios", QEMU_CAPS_SMBIOS_TYPE);
+ DO_TEST_PARSE_ERROR("smbios-date", QEMU_CAPS_SMBIOS_TYPE);
DO_TEST("watchdog", NONE);
DO_TEST("watchdog-device", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
--
Martin