SAK-40560: SiteStats allows mutation of cached event registry data (sakaiproject#5959)

* SAK-40560: SiteStats allows mutation of cached event registry data

* SAK-40560: implement Cloneable using copy constructor

Author: plukasew

sitestats/sitestats-api/src/java/org/sakaiproject/sitestats/api/event/EventInfo.java

@@ -20,7 +20,7 @@
 
 import java.io.Serializable;
 
-public class EventInfo implements Serializable {
+public class EventInfo implements Serializable, Cloneable {
 	private static final long	serialVersionUID	= 1L;
 	private String				eventId;
 	private boolean				selected;
@@ -30,6 +30,17 @@ public EventInfo(String eventId) {
 		this.eventId = eventId.trim();
 	}
 
+	public EventInfo(EventInfo info) {
+		eventId = info.eventId;
+		selected = info.selected;
+		anonymous = info.anonymous;
+	}
+
+	@Override
+	public EventInfo clone() {
+		return new EventInfo(this);
+	}
+
 	public String getEventId() {
 		return eventId;
 	}

sitestats/sitestats-api/src/java/org/sakaiproject/sitestats/api/event/ToolInfo.java

@@ -26,7 +26,7 @@
 import org.sakaiproject.sitestats.api.parser.EventParserTip;
 
 
-public class ToolInfo implements Serializable {
+public class ToolInfo implements Serializable, Cloneable {
 	private static final long			serialVersionUID	= 1L;
 	private String						toolId;
 	private List<String>				additionalToolIds;
@@ -44,6 +44,22 @@ public ToolInfo(String toolId, List<String> additionalToolIds) {
 		eventInfos = new ArrayList<EventInfo>();
 	}
 
+	public ToolInfo(ToolInfo tool) {
+		toolId = tool.toolId;
+		additionalToolIds = tool.additionalToolIds != null ? new ArrayList<>(tool.additionalToolIds) : null;
+		eventInfos = new ArrayList<>(tool.eventInfos.size());
+		for (EventInfo info : tool.eventInfos) {
+			eventInfos.add(info.clone());
+		}
+		selected = tool.selected;
+		eventParserTip = tool.eventParserTip != null ? tool.eventParserTip.clone() : null;
+	}
+
+	@Override
+	public ToolInfo clone() {
+		return new ToolInfo(this);
+	}
+
 	public List<EventInfo> getEvents() {
 		return eventInfos;
 	}
@@ -132,5 +148,4 @@ public String toString() {
 		}
 		return buff.toString();
 	}
-	
 }

sitestats/sitestats-api/src/java/org/sakaiproject/sitestats/api/parser/EventParserTip.java

@@ -20,7 +20,7 @@
 
 import java.io.Serializable;
 
-public class EventParserTip implements Serializable {
+public class EventParserTip implements Serializable, Cloneable {
 	private static final long	serialVersionUID	= 1L;
 	private String forWhat;
 	private String separator;
@@ -37,6 +37,17 @@ public EventParserTip(String forWhat, String separator, String index) {
 		this.index = index;
 	}
 
+	public EventParserTip(EventParserTip tip) {
+		forWhat = tip.forWhat;
+		separator = tip.separator;
+		index = tip.index;
+	}
+
+	@Override
+	public EventParserTip clone() {
+		return new EventParserTip(this);
+	}
+
 	public String getFor() {
 		return forWhat;
 	}

sitestats/sitestats-impl/src/java/org/sakaiproject/sitestats/impl/event/EventRegistryServiceImpl.java

@@ -402,9 +402,17 @@ private List<ToolInfo> getMergedEventRegistry() {
 		}
 		// STAT-380 ensure we do not return a null from this method
 		if (eventRegistry == null) {
-			eventRegistry = new ArrayList<ToolInfo>(0);
+			return new ArrayList<>(0);
 		}
-		return eventRegistry;
+
+		// defensively deep copy the event registry before returning it, this prevents outside code from
+		// mutating the cached registry entries
+		List<ToolInfo> cloneRegistry = new ArrayList<>(eventRegistry.size());
+		for (ToolInfo t : eventRegistry) {
+			cloneRegistry.add(t.clone());
+		}
+
+		return cloneRegistry;
 	}
 
 	/** Process event registry expired notifications */