Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 191100

Summary:	Figure.getBounds() doesn't make a defensive copy
Product:	[Tools] GEF	Reporter:	Mark Levison <mark.levison>
Component:	GEF-Legacy Draw2d	Assignee:	gef-inbox <gef-inbox>
Status:	RESOLVED INVALID	QA Contact:
Severity:	normal
Priority:	P3
Version:	3.2
Target Milestone:	---
Hardware:	PC
OS:	Windows XP
Whiteboard:

Description Mark Levison

2007-06-05 14:59:37 EDT

Build ID:  20070208-1315

Steps To Reproduce:
As implemented in 3.2 Figure.getBounds() - doesn't return a defensive copy. So if the caller modifies the rect in their paint method their screwed. In Effective Java Item 24 - Josh Bloch says: 
"You must program defensively with the assumption that clients of your class will do their best to destroy its invariants. This may actually be true if someone tries to break the security of your system, but more likely your class will have to cope with unexpected behavior resulting from honest mistakes on the part of the programmer using your API. Either way, it is worth taking the time to write classes that are robust in the face of ill-behaved clients."

I think this is possibly one of the most important rules in the book.


More information:

Comment 1 Anthony Hunter

2007-06-05 15:08:51 EDT

All of the examples do Figure.getBounds().getCopy().

Comment 2 Mark Levison

2007-06-05 15:15:22 EDT

I'm more than a bit surprised - however if thats the standard - I guess I will have to implement SafeFigure - with defensive getters.

Comment 3 Anthony Hunter

2007-06-05 16:25:22 EDT

The JavaDoc for Figure.getBounds() specifically says:

"callers of this method must not modify the returned Rectangle."

Comment 4 Pratik Shah

2007-06-06 13:27:25 EDT

Figure.getBounds() gets invoked a lot, often during painting (which needs to be quick).  And it's not always modified.  Essentially, this is a trade-off between performance and defense(?).  We chose performance, which makes sense for a framework.

Comment 5 Mark Levison

2007-06-06 13:33:07 EDT

Pratik - thanks for the comment. I will ask only one more question did anyone prove (using a profiler) that the defensive copy of Rectangle was really expensive? Since Java 1.3/1.4 I'm surprised how good the GC has become at making it very very cheap (effectively free) to make defensive copies. So I wonder if this was a premature optimization.

This all came up because I got burned - by accidentally modifying a Rectangle and that made me look at the source.

Comment 6 Randy Hudson

2007-06-06 15:33:09 EDT

> expensive? Since Java 1.3/1.4 I'm surprised how good the GC has become at
> making it very very cheap (effectively free) to make defensive copies. So I
> wonder if this was a premature optimization.

I suppose you might say that. The optimization was done "pre" the JVMs becoming "mature". Java 1.2 was the original target platform.