Bug 983885

Summary: There is no error raised in case of a string passed as nfs_timeo or nfs_retrans
Product: Red Hat Enterprise Virtualization Manager Reporter: Katarzyna Jachim <kjachim>
Component: ovirt-engine-restapiAssignee: Michael Pasternak <mpastern>
Status: CLOSED WONTFIX QA Contact: Elena <edolinin>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 3.2.0CC: abaron, abonas, acathrow, amureini, bazulay, iheim, mpastern, ncredi, oramraz, Rhev-m-bugs, srevivo
Target Milestone: ---   
Target Release: 3.4.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: storage
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-08-25 08:23:12 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Storage RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Attachments:
Description Flags
whole test logs (with vdsm & engine logs etc.) none

Description Katarzyna Jachim 2013-07-12 03:36:54 EDT
Created attachment 772566 [details]
whole test logs (with vdsm & engine logs etc.)

Description of problem:
Create request:
<storage_domain>
    <name>test_148670_str_retrans</name>
    <type>data</type>
    <storage>
        <nfs_timeo>720</nfs_timeo>
        <type>nfs</type>
        <nfs_retrans>a</nfs_retrans>
        <address>10.35.64.106</address>
        <path>/fastpass/jenkins-vm-28_nfs_2013_07_11_10_42_36_57100</path>
        <nfs_version>v3</nfs_version>
    </storage>
    <host>
        <name>10.35.160.49</name>
    </host>
    <storage_format>v3</storage_format>
</storage_domain>

Created storage domain:
<storage_domain href="/api/storagedomains/fac5190b-618c-41bc-9ec9-7b3a8fb894be" id="fac5190b-618c-41bc-9ec9-7b3a8fb894be">
    <name>test_148670_str_retrans</name>
    <link href="/api/storagedomains/fac5190b-618c-41bc-9ec9-7b3a8fb894be/permissions" rel="permissions"/>
    <link href="/api/storagedomains/fac5190b-618c-41bc-9ec9-7b3a8fb894be/disks" rel="disks"/>
    <type>data</type>
    <status>
        <state>unattached</state>
    </status>
    <master>false</master>
    <storage>
        <type>nfs</type>
        <address>10.35.64.106</address>
        <path>/fastpass/jenkins-vm-28_nfs_2013_07_11_10_42_36_57100</path>
        <nfs_version>v3</nfs_version>
        <nfs_timeo>720</nfs_timeo>
    </storage>
    <available>1972463730688</available>
    <used>191126044672</used>
    <committed>0</committed>
    <storage_format>v3</storage_format>
</storage_domain>

Incorrect parameter was just skipped, instead of raising an error.

Version-Release number of selected component (if applicable): sf18.3 and probably all earlier


How reproducible: 100%

Steps to Reproduce:
1. try to create a storage domain with nfs_timeo='a' or nfs_retrans='a'


Actual results:
Storage domain is created, invalid parameter is skipped


Expected results:
Operation should fail

Additional info:
Comment 1 Alissa 2013-08-22 07:09:33 EDT
The value is ignored before it reaches REST resource implementation, because when the object is received in the add method of the resource -it's already null, thus it probably happens when java object is created from the user's input.
The whiteboard storage should be probably changed to infra - this is not a storage issue. Ayal?
Comment 2 Michael Pasternak 2013-08-25 06:10:09 EDT
this is doesn't seems right, int parsing produce [1], if for some reason same
doesn't happen for the xs:unsignedShort you have used for the
nfs_timeo/nfs_retrans, please file bug for RestEasy and as workaround use
"xs:int" as a attribute type,

also to be on a safe side, make sure that passing legal value does work for you,
(i.e this is to understand if used attributes are not misplaced in the request body) 

[1] HTTP Status 400 - javax.xml.bind.UnmarshalException
Comment 3 Alissa 2013-08-25 06:32:12 EDT
The same behavior happens for int.
for example - try to change network's mtu property that is defined as xs:int.

If passing it double-quoted as string, it arrives to the rest resource as null, no 400 is raised.

Example - pass as string:
<network>
<mtu>"68"</mtu>
</network>

If passing it properly, it arrives OK as Integer object with the correct value.
Example - pass as int:
<network>
<mtu>68</mtu>
</network>

BTW, is the schema validation enabled? I didn't find where it is enabled in the code.
Comment 4 Michael Pasternak 2013-08-25 08:22:20 EDT
(In reply to Alissa from comment #3)
> The same behavior happens for int.
> for example - try to change network's mtu property that is defined as xs:int.
> 
> If passing it double-quoted as string, it arrives to the rest resource as
> null, no 400 is raised.
> 
> Example - pass as string:
> <network>
> <mtu>"68"</mtu>
> </network>
> 

jaxb unmarshalling works like this [1], DefaultValidationEventHandler
terminates the marshal operation after encountering either a fatal error or an
error, For a JAXB 2.0 client application, there is no explicitly defined
default validation handler and the default event handling only terminates the
marshal operation after encountering a fatal error.

so i'm not sure this is should be handled in the api, it is client 
serializing code prerogative, python-sdk serialization for instance does [2],
in strongly typed languages data-loss truncation is blocked by runtime errors,

if someone writes own serialization code, he should be considering schema types
restrictions,

in any case i don't think mentioned negative flows should be tested against
the api directly, therefore i would suggest closing this bug.

[1] https://jaxb.java.net/nonav/2.2.4/docs/api/javax/xml/bind/Unmarshaller.html#unmarshalByDeclaredType

[2]

[oVirt shell (connected)]# add vm --name test --cluster-name Default --template-name Blank --memory aaa

unknown error: %d format: a number is required, not str
Comment 5 Michael Pasternak 2013-08-25 08:27:06 EDT
(In reply to Michael Pasternak from comment #4)
> (In reply to Alissa from comment #3)
> > The same behavior happens for int.
> > for example - try to change network's mtu property that is defined as xs:int.
> > 
> > If passing it double-quoted as string, it arrives to the rest resource as
> > null, no 400 is raised.
> > 
> > Example - pass as string:
> > <network>
> > <mtu>"68"</mtu>
> > </network>
> > 
> 
> jaxb unmarshalling works like this [1], DefaultValidationEventHandler
> terminates the marshal operation after encountering either a fatal error or
> an
> error, For a JAXB 2.0 client application, there is no explicitly defined
> default validation handler and the default event handling only terminates the
> marshal operation after encountering a fatal error.
> 
> so i'm not sure this is should be handled in the api, it is client 
> serializing code prerogative, python-sdk serialization for instance does [2],
> in strongly typed languages data-loss truncation is blocked by runtime
> errors,
> 

accidentally mixed two different bugs in a same reply, 

s/data-loss truncation is blocked by runtime errors/type conversion is blocked by errors