Frank Kobzik has uploaded a new change for review. Change subject: engine: Fix NPEx vulnerabities related to osinfo change ......................................................................
engine: Fix NPEx vulnerabities related to osinfo change This patch fixes couple of NPEx vulnerabities related to ConsolesBase and OS info in AsyncDataProvider. The vulnerabities happened when changes in osinfo caused invalidity of VMs already stored in the database. The problem was fixed by introducing null checks in appropriate places and a map with the possibility of returning default value if requested key is not found. Change-Id: I04713a90374cac30fd358fdd0fcf02d2a020d477 Signed-off-by: Frantisek Kobzik <fkob...@redhat.com> Bug-Url: https://bugzilla.redhat.com/1046809 --- A frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/MapWithDefaults.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsoleModelsCache.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsolesBase.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java 5 files changed, 78 insertions(+), 30 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/09/23809/1 diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/MapWithDefaults.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/MapWithDefaults.java new file mode 100644 index 0000000..636c17c --- /dev/null +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/MapWithDefaults.java @@ -0,0 +1,51 @@ +package org.ovirt.engine.ui.uicommonweb; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +/** + * Simple map that supports default value when requested key doesn't exist. + * @param <K> key type + * @param <V> value type + */ +public class MapWithDefaults<K, V> { + + private Map<K, V> data; + private V defaultValue; + + public MapWithDefaults(Map<K, V> data) { + this.data = new HashMap<K, V>(); + this.data.putAll(data); + } + + /** + * Compulsory default constructor. + */ + public MapWithDefaults() { } + + /** + * This value will be returned when client passes non-existing key to get(). + */ + public void setDefaultValue(V defaultValue) { + this.defaultValue = defaultValue; + } + + /** + * Retrieves value under specific key. If the key doesn't exist, returns default value instead. + */ + public V get(K key) { + return (data.containsKey(key)) + ? data.get(key) + : defaultValue; + } + + /** + * Returns unmodifiable set of keys. + */ + public Set<K> keySet() { + return Collections.unmodifiableSet(data.keySet()); + } + +} diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java index 1843b8a..26d9191 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java @@ -127,6 +127,7 @@ import org.ovirt.engine.core.common.queries.gluster.GlusterServiceQueryParameters; import org.ovirt.engine.core.common.queries.gluster.GlusterVolumeAdvancedDetailsParameters; import org.ovirt.engine.core.common.queries.gluster.GlusterVolumeQueriesParameters; +import org.ovirt.engine.ui.uicommonweb.MapWithDefaults; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.utils.SimpleDependecyInjector; import org.ovirt.engine.core.compat.Guid; @@ -158,6 +159,8 @@ private static final String GENERAL = "general"; //$NON-NLS-1$ + public static final int DEFAULT_OS_ID = 0; + // dictionary to hold cache of all config values (per version) queried by client, if the request for them succeeded. private static HashMap<KeyValuePairCompat<ConfigurationValues, String>, Object> cachedConfigValues = new HashMap<KeyValuePairCompat<ConfigurationValues, String>, Object>(); @@ -168,7 +171,7 @@ private static String _defaultConfigurationVersion = null; // cached OS names - private static HashMap<Integer, String> osNames; + private static MapWithDefaults<Integer, String> osNames; // cached list of os ids private static List<Integer> osIds; @@ -192,9 +195,8 @@ // default OS per architecture private static HashMap<ArchitectureType, Integer> defaultOSes; - // cached os's support for display types (given compatibility version) - private static Map<Integer, Map<Version, List<DisplayType>>> displayTypes; + private static MapWithDefaults<Integer, Map<Version, List<DisplayType>>> displayTypes; public static String getDefaultConfigurationVersion() { return _defaultConfigurationVersion; @@ -1096,10 +1098,6 @@ IdQueryParameters params = new IdQueryParameters(vmId); params.setRefresh(isRefresh); Frontend.getInstance().runQuery(VdcQueryType.GetAllDisksByVmId, params, aQuery); - } - - public static HashMap<Integer, String> getOsNames() { - return osNames; } public static HashMap<Integer, String> getOsUniqueOsNames() { @@ -3323,7 +3321,9 @@ callback.asyncCallback = new INewAsyncCallback() { @Override public void onSuccess(Object model, Object returnValue) { - osNames = (HashMap<Integer, String>) ((VdcQueryReturnValue) returnValue).getReturnValue(); + HashMap<Integer, String> result = ((VdcQueryReturnValue) returnValue).getReturnValue(); + osNames = new MapWithDefaults<Integer, String>(result); + osNames.setDefaultValue(osNames.get(DEFAULT_OS_ID)); initOsIds(); } }; @@ -3351,6 +3351,10 @@ Frontend.getInstance().runQuery(VdcQueryType.OsRepository, new OsQueryParameters(OsRepositoryVerb.GetOsArchitectures), callback); } + public static boolean osNameExists(Integer osId) { + return osNames.keySet().contains(osId); + } + public static String getOsName(Integer osId) { // can be null as a consequence of setItems on ListModel if (osId == null) { @@ -3373,7 +3377,9 @@ callback.asyncCallback = new INewAsyncCallback() { @Override public void onSuccess(Object model, Object returnValue) { - displayTypes = ((VdcQueryReturnValue) returnValue).getReturnValue(); + Object result = ((VdcQueryReturnValue) returnValue).getReturnValue(); + displayTypes = new MapWithDefaults<Integer, Map<Version, List<DisplayType>>>((Map<Integer, Map<Version, List<DisplayType>>>) result); + displayTypes.setDefaultValue(displayTypes.get(DEFAULT_OS_ID)); } }; Frontend.getInstance().runQuery(VdcQueryType.OsRepository, new OsQueryParameters(OsRepositoryVerb.GetDisplayTypes), callback); diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsoleModelsCache.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsoleModelsCache.java index b8a6476..3a3471e 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsoleModelsCache.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsoleModelsCache.java @@ -57,20 +57,6 @@ */ public void updateVmCache(Iterable<VM> newItems) { updateCache(true, newItems); - Set<Guid> vmIds = new HashSet<Guid>(); - - if (newItems != null) { - for (VM vm : newItems) { - if (vmConsoles.containsKey(vm.getId())) { - vmConsoles.get(vm.getId()).setVm(vm); // only update vm - } else { - vmConsoles.put(vm.getId(), new VmConsolesImpl(vm, parentModel, consoleContext)); - } - vmIds.add(vm.getId()); - } - } - - vmConsoles.keySet().retainAll(vmIds); } public void updatePoolCache(Iterable<VM> poolRepresentants) { diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsolesBase.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsolesBase.java index dd450b2..42cb1ca 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsolesBase.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsolesBase.java @@ -48,7 +48,6 @@ setDefaultSelectedProtocol(); } - private void fillModels() { SpiceConsoleModel spiceConsoleModel = new SpiceConsoleModel(vm, parentModel); spiceConsoleModel.getErrorEvent().addListener(new ConsoleModelErrorEventListener(parentModel)); @@ -64,7 +63,6 @@ rdpConsoleModel.setForceVmStatusUp(myContext == ConsoleOptionsFrontendPersister.ConsoleContext.UP_BASIC); consoleModels.put(ConsoleProtocol.RDP, rdpConsoleModel); } - protected void setDefaultSelectedProtocol() { List<ConsoleProtocol> allProtocols = new ArrayList<ConsoleProtocol>(Arrays.asList(ConsoleProtocol.values())); @@ -83,10 +81,12 @@ } public boolean canSelectProtocol(ConsoleProtocol protocol) { - return consoleModels.get(protocol).canBeSelected(); + return (protocol == null) + ? false + : consoleModels.get(protocol).canBeSelected(); } - public void selectProtocol(ConsoleProtocol protocol) throws IllegalArgumentException { + public void selectProtocol(ConsoleProtocol protocol) throws IllegalArgumentException { if (!canSelectProtocol(protocol)) { throw new IllegalArgumentException("Cannot select " +protocol.toString() + " protocol for vm " + getVm().getName()); // $NON-NLS-1$ $NON-NLS-2$ } @@ -102,7 +102,9 @@ } public boolean canConnectToConsole() { - return consoleModels.get(selectedProtocol).canConnect(); + return (selectedProtocol == null) + ? false + : consoleModels.get(selectedProtocol).canConnect(); } public ConsoleOptionsFrontendPersister.ConsoleContext getConsoleContext() { diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java index 164633d..a2c0761 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java @@ -1550,8 +1550,11 @@ { defaultHost_SelectedItemChanged(sender, args); } - else if (sender == getOSType()) - { + else if (sender == getOSType()) { + if (!AsyncDataProvider.osNameExists(getOSType().getSelectedItem())) { + getOSType().setSelectedItem(AsyncDataProvider.DEFAULT_OS_ID); + } + oSType_SelectedItemChanged(sender, args); getVmInitModel().osTypeChanged(getOSType().getSelectedItem()); updateDisplayProtocol(); -- To view, visit http://gerrit.ovirt.org/23809 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I04713a90374cac30fd358fdd0fcf02d2a020d477 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <fkob...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
