Tomas Jelinek has uploaded a new change for review. Change subject: frontend: NPE in Import*Model ......................................................................
frontend: NPE in Import*Model There was a NPE vulnerability in ImportTemplateModel and ImportVmModel. The problem was that the VmBackupModel.restore() called first the setItems() and than init(). The setItems have done some async call and set the result to the super.setItems(), while the init() already needed this in initDisksStorageDomainsList() method (which is also called in a callback). So if the callback from setItems returned sooner than the ones in init() it worked. If not, the initDisksStorageDomainsList failed on NPE. Fixed by calling the real init() logic only when the data are really prepared. Change-Id: Id97c8368ecf80ceb7ef1aa456d32a022dd5043bb Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1019862 Signed-off-by: Tomas Jelinek <tjeli...@redhat.com> --- M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/ImportTemplateModel.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmModel.java 3 files changed, 68 insertions(+), 63 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/43/20243/1 diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java index 355626b..7887366 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/VmBackupModel.java @@ -186,8 +186,7 @@ tempVar3.setTitle(ConstantsManager.getInstance().getConstants().cancel()); tempVar3.setIsCancel(true); model.getCommands().add(tempVar3); - model.setItems(getSelectedItems()); - model.init(getEntity().getId()); + model.init(getSelectedItems(), getEntity().getId()); // Add 'Close' command UICommand closeCommand = new UICommand("Cancel", this); //$NON-NLS-1$ diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/ImportTemplateModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/ImportTemplateModel.java index 4e30730..872a925 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/ImportTemplateModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/ImportTemplateModel.java @@ -10,6 +10,7 @@ import org.ovirt.engine.core.common.queries.SearchParameters; import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; import org.ovirt.engine.core.common.queries.VdcQueryType; +import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.ui.frontend.AsyncQuery; import org.ovirt.engine.ui.frontend.Frontend; import org.ovirt.engine.ui.frontend.INewAsyncCallback; @@ -50,7 +51,7 @@ } @Override - public void setItems(final Iterable value) + public void setItems(final Iterable value, final Guid storageDomainId) { String vmt_guidKey = "_VMT_ID ="; //$NON-NLS-1$ String orKey = " or "; //$NON-NLS-1$ @@ -92,6 +93,7 @@ templateDataList.add(templateData); } ImportTemplateModel.super.setSuperItems(templateDataList); + doInit(storageDomainId); } })); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmModel.java index 7b7baae..4ebc11b4 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmModel.java @@ -156,69 +156,73 @@ getClusterQuota().setIsAvailable(false); } - public void init(Guid storageDomainId) { - // get Storage pool - AsyncDataProvider.getDataCentersByStorageDomain(new AsyncQuery(this, new INewAsyncCallback() { + public void init(List items, final Guid storageDomainId) { + setItems(items, storageDomainId); + } - @Override - public void onSuccess(Object model, Object returnValue) { - ArrayList<StoragePool> pools = (ArrayList<StoragePool>) returnValue; - if (pools == null || pools.size() != 1) { - return; - } + protected void doInit(final Guid storageDomainId) { + // get Storage pool + AsyncDataProvider.getDataCentersByStorageDomain(new AsyncQuery(this, new INewAsyncCallback() { - StoragePool dataCenter = pools.get(0); - setStoragePool(dataCenter); - // show quota - if (dataCenter != null && dataCenter.getQuotaEnforcementType() != QuotaEnforcementTypeEnum.DISABLED) { - hasQuota = true; - } - if (hasQuota) { - getClusterQuota().setIsAvailable(true); - getCluster().getSelectedItemChangedEvent().addListener(quotaClusterListener); - } - // get cluster - if (dataCenter != null) { - AsyncDataProvider.getClusterByServiceList(new AsyncQuery(ImportVmModel.this, new INewAsyncCallback() { - @Override - public void onSuccess(Object model, Object returnValue) { - ArrayList<VDSGroup> clusters = (ArrayList<VDSGroup>) returnValue; - getCluster().setItems(clusters); - getCluster().setSelectedItem(Linq.firstOrDefault(clusters)); - // get storage domains - AsyncDataProvider.getStorageDomainList(new AsyncQuery(ImportVmModel.this, - new INewAsyncCallback() { + @Override + public void onSuccess(Object model, Object returnValue) { + ArrayList<StoragePool> pools = (ArrayList<StoragePool>) returnValue; + if (pools == null || pools.size() != 1) { + return; + } - @Override - public void onSuccess(Object model, Object returnValue) { - ArrayList<StorageDomain> storageDomains = - (ArrayList<StorageDomain>) returnValue; - // filter storage domains - filteredStorageDomains = - new ArrayList<StorageDomain>(); - for (StorageDomain domain : storageDomains) { - if (Linq.isDataActiveStorageDomain(domain)) { - filteredStorageDomains.add(domain); - } - } + StoragePool dataCenter = pools.get(0); + setStoragePool(dataCenter); + // show quota + if (dataCenter != null && dataCenter.getQuotaEnforcementType() != QuotaEnforcementTypeEnum.DISABLED) { + hasQuota = true; + } + if (hasQuota) { + getClusterQuota().setIsAvailable(true); + getCluster().getSelectedItemChangedEvent().addListener(quotaClusterListener); + } + // get cluster + if (dataCenter != null) { + AsyncDataProvider.getClusterByServiceList(new AsyncQuery(ImportVmModel.this, new INewAsyncCallback() { + @Override + public void onSuccess(Object model, Object returnValue) { + ArrayList<VDSGroup> clusters = (ArrayList<VDSGroup>) returnValue; + getCluster().setItems(clusters); + getCluster().setSelectedItem(Linq.firstOrDefault(clusters)); + // get storage domains + AsyncDataProvider.getStorageDomainList(new AsyncQuery(ImportVmModel.this, + new INewAsyncCallback() { - getStorage().setItems(filteredStorageDomains); - if (hasQuota) { - initQuotaForStorageDomains(); - } else { - initDisksStorageDomainsList(); - } - } + @Override + public void onSuccess(Object model, Object returnValue) { + ArrayList<StorageDomain> storageDomains = + (ArrayList<StorageDomain>) returnValue; + // filter storage domains + filteredStorageDomains = + new ArrayList<StorageDomain>(); + for (StorageDomain domain : storageDomains) { + if (Linq.isDataActiveStorageDomain(domain)) { + filteredStorageDomains.add(domain); + } + } - }), - getStoragePool().getId()); - } - }), - dataCenter.getId(), true, false); - } - } - }), - storageDomainId); + getStorage().setItems(filteredStorageDomains); + if (hasQuota) { + initQuotaForStorageDomains(); + } else { + initDisksStorageDomainsList(); + } + } + + }), + getStoragePool().getId()); + } + }), + dataCenter.getId(), true, false); + } + } + }), + storageDomainId); } private void initQuotaForStorageDomains() { @@ -524,8 +528,7 @@ && getClusterQuota().getIsValid(); } - @Override - public void setItems(final Iterable value) + public void setItems(final Iterable value, final Guid storageDomainId) { String vm_guidKey = "ID ="; //$NON-NLS-1$ String orKey = " or "; //$NON-NLS-1$ @@ -568,6 +571,7 @@ vmDataList.add(vmData); } ImportVmModel.super.setItems(vmDataList); + doInit(storageDomainId); } })); -- To view, visit http://gerrit.ovirt.org/20243 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id97c8368ecf80ceb7ef1aa456d32a022dd5043bb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
