Bug 1412749 - [engine-webadmin] Uncaught exception is received when trying to create NFS domain with wrong value type in 'Custom Connection Parameters'
Summary: [engine-webadmin] Uncaught exception is received when trying to create NFS do...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: ovirt-engine
Classification: oVirt
Component: Frontend.Core
Version: 4.1.0
Hardware: x86_64
OS: Unspecified
medium
medium
Target Milestone: ovirt-4.1.4
: 4.1.4.1
Assignee: Tal Nisan
QA Contact: samuel macko
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-01-12 17:00 UTC by Elad
Modified: 2017-07-28 14:18 UTC (History)
9 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-07-28 14:18:52 UTC
oVirt Team: Storage
Embargoed:
rule-engine: ovirt-4.1+
lsvaty: testing_ack+


Attachments (Terms of Use)
console.log, engine.log, server.log (4.41 MB, application/x-gzip)
2017-01-12 17:00 UTC, Elad
no flags Details
engine.log, server.log, ui.log (2) (1.17 MB, application/x-gzip)
2017-02-13 09:34 UTC, Elad
no flags Details
engine.log in DEBUG (3) (13.58 MB, application/x-gzip)
2017-02-13 14:44 UTC, Elad
no flags Details
nfs dialog master (50.10 KB, image/png)
2017-05-21 18:06 UTC, Greg Sheremeta
no flags Details


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 78503 0 master MERGED webadmin: Mark illegal input in NFS storage popup advance fields 2020-02-14 05:31:59 UTC
oVirt gerrit 78711 0 ovirt-engine-4.1 MERGED webadmin: Mark illegal input in NFS storage popup advance fields 2020-02-14 05:31:59 UTC

Description Elad 2017-01-12 17:00:41 UTC
Created attachment 1240029 [details]
console.log, engine.log, server.log

Description of problem:
I got an Uncaught exception in UI when tried to create NFS domain with string in 'Retransmissions (#)' field.

Version-Release number of selected component (if applicable):
ovirt-engine-4.1.0-0.4.master.20170111194730.git0a1100c.el7.centos.noarch

How reproducible:
Always

Steps to Reproduce:
1. Create NFS domain with string in 'Retransmissions (#)' text box


Actual results:
UI.log:

2017-01-12 18:51:05,583+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-2) [] Permutation name: 0661FF244C5EA5656FC338D0C4E7609A
2017-01-12 18:51:05,584+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-2) [] Uncaught exception: com.google.gwt.event.shared.UmbrellaException: Exception caught: (TypeError) 
 __gwt$exception: <skipped>: Cannot read property 'cc' of undefined
        at Unknown.at bz(Unknown Source)
        at Unknown.at Object.xv(Unknown Source)
        at Unknown.at Object.Fv(Unknown Source)
        at Unknown.at Object._8(Unknown Source)
        at Unknown.at Object.d9(Unknown Source)
        at Unknown.at l8(Unknown Source)
        at Unknown.at Vp(Unknown Source)
        at Unknown.at Object.dq(Unknown Source)
        at Unknown.at V3(Unknown Source)
        at Unknown.at Xp(Unknown Source)
Caused by: com.google.gwt.core.client.JavaScriptException: (TypeError) 
 __gwt$exception: <skipped>: Cannot read property 'cc' of undefined
        at Unknown.anonymous(Unknown Source)
        at Unknown.at Object.g3(Unknown Source)
        at Unknown.at oQ(Unknown Source)
        at Unknown.at Object.q3(Unknown Source)
        at Unknown.at Object.sKm [as Zf](Unknown Source)
        at Unknown.at Object.Q5(Unknown Source)
        at Unknown.at Object.P3(Unknown Source)
        at Unknown.at v8(Unknown Source)
        at Unknown.at l8(Unknown Source)
        at Unknown.at Vp(Unknown Source)
        at Unknown.at Object.dq(Unknown Source)


Expected results:
No exception


Additional info:
console.log, engine.log, server.log

Comment 1 Yaniv Kaul 2017-02-12 15:28:26 UTC
Elad - can this be reproduced with debuginfo? Otherwise, the stack trace is obfuscated. It has to match the relevant webadmin package version - and you'll get it in ui.log.

Comment 2 Elad 2017-02-13 09:34:27 UTC
Created attachment 1249784 [details]
engine.log, server.log, ui.log (2)

Modified "<!-- Loggers for the UI -->" in /usr/share/ovirt-engine/services/ovirt-engine/ovirt-engine.xml.in to DEBUG.

I get the same exception in ui.log:

2017-02-13 11:28:15,637+02 INFO  [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-3) [] GWT symbolmaps are not installed, please install them to de-obfuscate the UI stack traces
2017-02-13 11:28:15,648+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-3) [] Permutation name: F13FE4CACE40D24F999C20570C1DE9D3
2017-02-13 11:28:15,648+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-3) [] Uncaught exception: com.google.gwt.event.shared.UmbrellaException: Exception caught: (TypeError) 
 __gwt$exception: <skipped>: Cannot read property 'cc' of undefined
        at Unknown.Av(webadmin-0.js@29429)
        at Unknown.Iv(webadmin-0.js@41)
        at Unknown.l9(webadmin-0.js@19)
        at Unknown.p9(webadmin-0.js@19)
        at Unknown.x8(webadmin-0.js@117)
        at Unknown.Wp(webadmin-0.js@26)
        at Unknown.eq(webadmin-0.js@24224)
        at Unknown.f4(webadmin-0.js@149)
        at Unknown.Yp(webadmin-0.js@112)
        at Unknown.vAf(webadmin-0.js@28145)
        at Unknown.f4e(webadmin-0.js@85)
        at Unknown.h6e(webadmin-0.js@46)
        at Unknown.Qx(webadmin-0.js@29)
        at Unknown.Ux(webadmin-0.js@57)
        at Unknown.eval(webadmin-0.js@54)
Caused by: com.google.gwt.core.client.JavaScriptException: (TypeError) 
 __gwt$exception: <skipped>: Cannot read property 'cc' of undefined
        at Unknown.s3(webadmin-0.js@28)
        at Unknown.IQ(webadmin-0.js@28)
        at Unknown.C3(webadmin-0.js@198)
        at Unknown.cJm(webadmin-0.js@10468)
        at Unknown.a6(webadmin-0.js@2463)
        at Unknown._3(webadmin-0.js@858)
        at Unknown.H8(webadmin-0.js@209)
        at Unknown.x8(webadmin-0.js@59)
        ... 10 more


rhevm-4.1.0.4-0.1.el7.noarch
ovirt-engine-webadmin-portal-4.1.0.4-0.1.el7.noarch

Comment 3 Yaniv Kaul 2017-02-13 11:00:16 UTC
Elad - are you sure the debuginfo version matches the webadmin package? (and restart engine after you've verified they are the same)

Comment 4 Elad 2017-02-13 14:44:57 UTC
Created attachment 1249874 [details]
engine.log in DEBUG (3)

Installed ovirt-engine-webadmin-portal-debuginfo, reproduced again:

2017-02-13 16:34:09,612+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-13) [] Permutation name: F13FE4CACE40D24F999C20570C1DE9D3
2017-02-13 16:34:09,614+02 ERROR [org.ovirt.engine.ui.frontend.server.gwt.OvirtRemoteLoggingService] (default task-13) [] Uncaught exception: com.google.gwt.event.shared.UmbrellaException: Exception caught: (TypeError) 
 __gwt$exception: <skipped>: Cannot read property 'cc' of undefined
        at java.lang.Throwable.fillInStackTrace(Throwable.java:114) [rt.jar:1.8.0_121]
        at java.lang.RuntimeException.RuntimeException(RuntimeException.java:32) [rt.jar:1.8.0_121]
        at com.google.web.bindery.event.shared.UmbrellaException.UmbrellaException(UmbrellaException.java:70) [gwt-servlet.jar:]
        at com.google.gwt.event.shared.UmbrellaException.UmbrellaException(UmbrellaException.java:25) [gwt-servlet.jar:]
        at com.google.gwt.event.shared.HandlerManager.$fireEvent(HandlerManager.java:117) [gwt-servlet.jar:]
        at com.google.gwt.user.client.ui.Widget.$fireEvent(Widget.java:127) [gwt-servlet.jar:]
        at com.google.gwt.user.client.ui.Widget.fireEvent(Widget.java:127) [gwt-servlet.jar:]
        at com.google.gwt.event.dom.client.DomEvent.fireNativeEvent(DomEvent.java:110) [gwt-servlet.jar:]
        at com.google.gwt.user.client.ui.Widget.$onBrowserEvent(Widget.java:163) [gwt-servlet.jar:]
        at com.google.gwt.user.client.ui.ValueBoxBase.onBrowserEvent(ValueBoxBase.java:240) [gwt-servlet.jar:]
        at com.google.gwt.user.client.DOM.dispatchEvent(DOM.java:1648) [gwt-servlet.jar:]
        at com.google.gwt.user.client.impl.DOMImplStandard.dispatchEvent(DOMImplStandard.java:320) [gwt-servlet.jar:]
        at com.google.gwt.core.client.impl.Impl.apply(Impl.java:296) [gwt-servlet.jar:]
        at com.google.gwt.core.client.impl.Impl.entry0(Impl.java:335) [gwt-servlet.jar:]
        at Unknown.eval(webadmin-0.js@54)
Caused by: com.google.gwt.core.client.JavaScriptException: (TypeError) 
 __gwt$exception: <skipped>: Cannot read property 'cc' of undefined
        at com.google.gwt.editor.client.impl.SimpleError.SimpleError(SimpleError.java:33) [gwt-servlet.jar:]
        at com.google.gwt.editor.client.impl.AbstractEditorDelegate.$recordError(AbstractEditorDelegate.java:133) [gwt-servlet.jar:]
        at com.google.gwt.editor.ui.client.adapters.ValueBoxEditor.getValue(ValueBoxEditor.java:82) [gwt-servlet.jar:]
        at org.ovirt.engine.ui.common.editor.UiCommonEditorVisitor$2.onKeyPress(UiCommonEditorVisitor.java:86)
        at com.google.gwt.event.dom.client.KeyPressEvent.dispatch(KeyPressEvent.java:78) [gwt-servlet.jar:]
        at com.google.gwt.event.shared.GwtEvent.dispatch(GwtEvent.java:76) [gwt-servlet.jar:]
        at com.google.web.bindery.event.shared.SimpleEventBus.$doFire(SimpleEventBus.java:173) [gwt-servlet.jar:]
        ... 11 more






rhevm-4.1.0.4-0.1.el7.noarch
ovirt-engine-webadmin-portal-debuginfo-4.1.0.4-0.1.el7.noarch

Comment 5 Tal Nisan 2017-05-17 13:46:04 UTC
This problem seems to be existing in many places in the webadmin, an input field is defined an ShortEntityModelTextBoxEditor/IntegerEntityModelTextBoxEditor and when a non number value is entered in the input a validation failure appears in the dialog yet the entity within the model is being set to null as the editor tries to parse the value to a short/int and does not succeed.
Since the model gets null in that field, the validation succeeds and the save action is executed and the wrong value in the field is being completely ignored.
Vojtech, Greg, what do you suggest to do in that case? I've checked and it also happens in the VM popup when trying to set illegal values in the RNG device

Comment 6 Greg Sheremeta 2017-05-21 18:06:40 UTC
Created attachment 1280787 [details]
nfs dialog master

Comment 7 Greg Sheremeta 2017-05-21 19:47:12 UTC
(In reply to Tal Nisan from comment #5)
> This problem seems to be existing in many places in the webadmin, an input
> field is defined an
> ShortEntityModelTextBoxEditor/IntegerEntityModelTextBoxEditor and when a non
> number value is entered in the input a validation failure appears in the
> dialog yet the entity within the model is being set to null as the editor
> tries to parse the value to a short/int and does not succeed.
> Since the model gets null in that field, the validation succeeds and the
> save action is executed and the wrong value in the field is being completely
> ignored.
> Vojtech, Greg, what do you suggest to do in that case? I've checked and it
> also happens in the VM popup when trying to set illegal values in the RNG
> device

It's a obviously a bug that the parse failure allows a form to submit. IMO that's what should be fixed. I'm not sure of where in the framework that change goes. @Vojtech or Alexander?

Comment 8 Greg Sheremeta 2017-05-21 19:56:18 UTC
[btw, I duplicated the validation failure bug on master, but I didn't get a JavaScript error. See screenshot.]

Comment 9 Alexander Wels 2017-05-22 12:10:13 UTC
Well the proper fix is to fix the model validator to not allow null values for the integer field. Tal hit the problem right on the head. The problem is that the integer parser fails to parse the value (since its not an integer) and a null is passed to the model as a result (I tried really hard to modify this behavior but couldn't).

A while ago I implemented some code to properly give an error in the UI in this particular case, however if the model validator itself allows the null to be properly validated, then the model is at fault. Simply fixing the MODEL validator to not allow null will solve the issue. Btw the model should have never allowed null to begin with if it is not a valid value to pass to the backend regardless of if the UI gives an error.

Comment 10 Vojtech Szocs 2017-05-22 17:40:25 UTC
When sync'ing the model (in this case, NfsStorageModel) from UI editor widgets, UiCommonEditorVisitor#setInModel is called.

Check out the GWT-generated class

  NfsStorageView_retransmissions_entity_Context.java

located under webadmin/gen directory. This is the class behind `ctx` param in UiCommonEditorVisitor#setInModel method when dealing with `Retransmissions (#)` input field.

The actual model-bound GWT widget is EntityModelTextBox (extends GWT ValueBox). The GWT editor (ShortEntityModelTextBoxEditor) sets it up with custom renderer & parser.

The general contract is that the parser (e.g. ToShortEntityModelParser) returns null on empty input and throws exception on invalid input.

> when a non number value is entered in the input a validation failure appears in the dialog yet the entity within the model is being set to null as the editor tries to parse the value to a short/int and does not succeed.

Check out EntityModelTextBox#getValue override.

In GWT editor framework, the general HasValue#getValue contract is that both empty and invalid value is treated as `null`. When sync'ing the model from UI editor widgets, we should adapt our models accordingly, as they are the ones handling the validation.

> Since the model gets null in that field, the validation succeeds and the save action is executed and the wrong value in the field is being completely ignored.

What happens is the following call

  // see NfsStorageView_retransmissions_entity_Context
  parent.getRetransmissions().setEntity(null)

due to invalid UI input. As I wrote above, both empty input and invalid input are represented by the same `null` value.

> Well the proper fix is to fix the model validator to not allow null values for the integer field.

Empty input is valid input for UI fields whose values are optional. That's the main reason (I think) why the model validator contract (IValidation) allows nulls.

Our problem is the lack of differentiation between empty vs. invalid input, both treated as `null`, which makes our validators think the input is empty and therefore passes the validation logic.

One solution is to augment HasEditorValidityState interface with method like:

  willStateBeValid(T value)

and modify UiCommonEditorVisitor#setInModel to be like:

  if (editorIsValidNow && editorWillBeValid) {
    // proceed to call ctx.setInModel
  }

that is the infra-level solution.

Another solution is to do fix this on model-level, however I'd prefer an infra-level fix to address all the models (scope == any editor implementing HasEditorValidityState).

Comment 11 Vojtech Szocs 2017-05-22 18:14:28 UTC
I've discussed this with Alex, basically UI infra (editor framework) currently sets the EntityModel's value to null if the user-entered input is either empty or invalid.

Most validators (including IntegerValidation) pass on null value. This is by design, as null generally means "missing value".

Perhaps NfsStorageModel#validate should use NotNullIntegerValidation instead?

  getRetransmissions().validateEntity(new IValidation[] {
    new NotNullIntegerValidation(0, RETRANS_MAX)
  });

Tal, what do you think?

Comment 12 Vojtech Szocs 2017-05-22 18:18:13 UTC
> This problem seems to be existing in many places in the webadmin

Can we identify those places? We should fix all of them.

> and the save action is executed and the wrong value in the field is being completely ignored.

Forgot to mention this - the "wrong value" translates to null which passes validation. This should be OK from infra perspective.. in such cases, model's validation should reject nulls, e.g. using NotNullIntegerValidation.

Comment 13 Tal Nisan 2017-05-23 11:25:01 UTC
I forgot to add as I thought it was obvious, I was referring to optional values that the user can enter i.e. null is allowed and if a value is entered it's supposed to be validated so in that case not null validation will not work

Comment 14 Alexander Wels 2017-05-23 12:03:03 UTC
Okay now I am confused. From the MODELs perspective what is the difference between the value being null from the user not entering a value and the value being null from the user entering an invalid value? It appears you are saying that in the second case somehow we have an exception when loading the model from the backend after its been saved while we don't in the first case? I don't understand how either case is any different from the model perspective.

Comment 15 Tal Nisan 2017-05-23 14:22:23 UTC
Let's take for instance the retransmissions number when creating a new NFS domain, you can leave it as null which is completely OK, the value will just not be used when sending the mount command or you can enter a custom value which should be a valid number.
What happens de facto is if you enter a value of "300," by mistake when you meant to enter 300, instead of the dialog notifying you that you entered an invalid value, the new connection will be saved without a custom number of retransmissions

Comment 16 Alexander Wels 2017-06-13 12:40:00 UTC
If you entere 3oo instead of 300 the field will be marked as an error and if the dialog allows a submission, then that is a bug in the dialog the infra structure will have marked the field in error. I know this works as for instance in the fence agent edit dialog you do this it will not allow you to save.

So I don't know what you want us to do? The infra will indicate the field is in error, the dialog validation ignores it and grabs the value (which will be null since it can't parse the value), and that happens to be a valid value for the field.

Comment 17 samuel macko 2017-07-14 08:10:06 UTC
Verified in ovirt-engine-4.1.4.1-0.1.el7.noarch


Note You need to log in before you can comment on or make changes to this bug.