GPAUDITLOGGING-49: potential NPE in AuditLogListener when trying to create unmodifiable list with null snapshot

[robertoschwald]

Original Reporter: sp
Environment: Grails 2.3.4
Version: Grails-AuditLogging 0.5.5.3
Migrated From: http://jira.grails.org/browse/GPAUDITLOGGING-49

In the below code oldMap[keyName] = Collections.unmodifiableList((List) snapshot); will throw an exception in the case when the snapshot is null

{code}
private populateOldStateMap(def oldState, Map oldMap, String keyName, int index) {
def oldPropertyState = oldState[index]
if (oldPropertyState instanceof PersistentCollection) {
PersistentCollection pc = (PersistentCollection) oldPropertyState;
PersistenceContext context = sessionFactory.getCurrentSession().getPersistenceContext();
CollectionEntry entry = context.getCollectionEntry(pc);
Object snapshot = entry.getSnapshot();
if (pc instanceof List) {
oldMap[keyName] = Collections.unmodifiableList((List) snapshot);
} else if (pc instanceof Map) {
oldMap[keyName] = Collections.unmodifiableMap((Map) snapshot);
} else if (pc instanceof Set) {
//Set snapshot is actually stored as a Map
if (snapshot) {
Map snapshotMap = (Map) snapshot;
oldMap[keyName] = Collections.unmodifiableSet(new HashSet(snapshotMap?.values()));
log.trace(oldMap[keyName].class);
} else {
log.trace("Cannot get snapshot of $pc Entry: $entry for keyName: $keyName");
oldMap[keyName] = null
}
} else {
oldMap[keyName] = pc;
}
} else {
oldMap[keyName] = oldPropertyState
}
}

{code}

[robertoschwald]

roos said:
Sure? There's a

if (snapshot) {...} before...

[robertoschwald]

sp said:
Hi Robert, thank you for your fast reaction :-).

The check against null is only done for the Set, but not List and Map. That's why I wrote that oldMap[keyName] = Collections.unmodifiableList((List) snapshot) can throw an exception.

[robertoschwald]

roos said:
As we moved to Grails Gorm events with the 1.0.x versions, I'm wondering if this still needs to be fixed in the old 0.5.x code base? Anyone still using the old one?