[Geoserver-devel] ModiifcationProxy does not prevent changes to collection mutable items

Hi,
I was playing around with the INSPIRE extension and I’ve noticed this issue.
Basically, ModificationProxy fails to guard against modification of collections, when the modification hits a mutable item contained in it.

ModificationProxy collection handling basically clones then collection before returning it, so that the caller works against a copy, but fails to deep clone it,
as a result:

  • if the client adds/removes items from the returned collection nothing bad happens
  • if the client grabs an item from the collection and alters it, it’s actually modifying the same item referred by the original collection

One example where this is happening in our GUI is the metadata links.
Here is a little experiment:

  1. Go to a layer, add a new metadata link with url “http://www.test.com”, save the layer.
  2. Open the same layer, modify the url to be “http://www.test.com/123
  3. Add a new link (this will force wicket to partially submit the form and thus change the state of the objects behind the GUI, which are coming from the ModificationProxy anyways)
  4. Exit by pressing cancel
  5. Open back the same layer and observe the results. The new link is not there, but the url of the first one got modified.

I haven’t tried, but guess anything that’s mutable and stuck in the metadata map will follow the same destiny (e.g., SQL view definitions).

Now… what to do about it.
We cannot wrap the collection contents in other ModificationProxies, because we don’t have a target interface for them.
I guess we can deep clone them. Deep cloning is not simple, and one might need some way to control how to stop the cloning at some point (so cloning by serialization is out of the question).
I’ve found this library that seems to be doing a decent job at deep cloning in memory with some cloning control: https://code.google.com/p/cloning/wiki/Usage

The library is also quite small. The idea is that we’d use a Spring registered Cloner object to deep clone the contents of all collections before returning them out of a modification proxy.

What do you think?

Cheers
Andrea

PS: I know this is yet another band aid, however I don’t see any good solution in sight until we can replace the whole config subsystem with a storage that’s not memory based and understands transactions.
At the moment I’m just trying to get the INSPIRE piugin going, not trying to start a crusade to improve the config subsytem…

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


On Sun, Apr 28, 2013 at 2:03 AM, Andrea Aime
<andrea.aime@anonymised.com>wrote:

Hi,
I was playing around with the INSPIRE extension and I've noticed this
issue.
Basically, ModificationProxy fails to guard against modification of
collections, when the modification hits a mutable item contained in it.

ModificationProxy collection handling basically clones then collection
before returning it, so that the caller works against a copy, but fails to
deep clone it,
as a result:
* if the client adds/removes items from the returned collection nothing
bad happens
* if the client grabs an item from the collection and alters it, it's
actually modifying the same item referred by the original collection

One example where this is happening in our GUI is the metadata links.
Here is a little experiment:
1) Go to a layer, add a new metadata link with url "http://www.test.com",
save the layer.
2) Open the same layer, modify the url to be "http://www.test.com/123&quot;
3) Add a new link (this will force wicket to partially submit the form and
thus change the state of the objects behind the GUI, which are coming from
the ModificationProxy anyways)
4) Exit by pressing cancel
5) Open back the same layer and observe the results. The new link is not
there, but the url of the first one got modified.

I haven't tried, but guess anything that's mutable and stuck in the
metadata map will follow the same destiny (e.g., SQL view definitions).

Now... what to do about it.
We cannot wrap the collection contents in other ModificationProxies,
because we don't have a target interface for them.
I guess we can deep clone them. Deep cloning is not simple, and one might
need some way to control how to stop the cloning at some point (so cloning
by serialization is out of the question).
I've found this library that seems to be doing a decent job at deep
cloning in memory with some cloning control:
https://code.google.com/p/cloning/wiki/Usage

The library is also quite small. The idea is that we'd use a Spring
registered Cloner object to deep clone the contents of all collections
before returning them out of a modification proxy.

Hmmm... i am kind of hesitant to add a new library dependency at such a
core level. Thinking, what about using annotations for this? Basically we
could come up with something that looked like this:

LayerInfo {

  @Proxy(target=MetadataLinkInfo.class)
  List<MetadataLinkinfo> getMetadataLinks();

}

Which could then be recognized by ModProxy and an appropriate collection
wrapper wrapping individual elements in a proxy could be implemented.

What do you think?

Cheers
Andrea

PS: I know this is yet another band aid, however I don't see any good
solution in sight until we can replace the whole config subsystem with a
storage that's not memory based and understands transactions.
At the moment I'm just trying to get the INSPIRE piugin going, not trying
to start a crusade to improve the config subsytem...

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_apr
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

On Tue, Apr 30, 2013 at 5:26 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

The library is also quite small. The idea is that we'd use a Spring
registered Cloner object to deep clone the contents of all collections
before returning them out of a modification proxy.

Hmmm... i am kind of hesitant to add a new library dependency at such a core
level. Thinking, what about using annotations for this? Basically we could
come up with something that looked like this:

LayerInfo {

  @Proxy(target=MetadataLinkInfo.class)
  List<MetadataLinkinfo> getMetadataLinks();

}

Which could then be recognized by ModProxy and an appropriate collection
wrapper wrapping individual elements in a proxy could be implemented.

That would work for MetadataLinkInfo, however, I don't believe it
would be applicable
to VirtualTable and the INSPIRE module UniqueResourceIdentifiers, which are both
classes.
We cannot proxy them using ModificationProxy as far as I know...

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

Bump :wink:

···

On Tue, Apr 30, 2013 at 5:49 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Apr 30, 2013 at 5:26 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

The library is also quite small. The idea is that we’d use a Spring
registered Cloner object to deep clone the contents of all collections
before returning them out of a modification proxy.

Hmmm… i am kind of hesitant to add a new library dependency at such a core
level. Thinking, what about using annotations for this? Basically we could
come up with something that looked like this:

LayerInfo {

@Proxy(target=MetadataLinkInfo.class)
List getMetadataLinks();

}

Which could then be recognized by ModProxy and an appropriate collection
wrapper wrapping individual elements in a proxy could be implemented.

That would work for MetadataLinkInfo, however, I don’t believe it
would be applicable
to VirtualTable and the INSPIRE module UniqueResourceIdentifiers, which are both
classes.
We cannot proxy them using ModificationProxy as far as I know…

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


Final bump. This is a real problem breaking at least two modules, if I don’t get any further feedback I’ll go ahead with the initially proposed solution

Cheers
Andrea

···

On Mon, May 6, 2013 at 10:38 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Bump :wink:

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


On Tue, Apr 30, 2013 at 5:49 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Apr 30, 2013 at 5:26 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

The library is also quite small. The idea is that we’d use a Spring
registered Cloner object to deep clone the contents of all collections
before returning them out of a modification proxy.

Hmmm… i am kind of hesitant to add a new library dependency at such a core
level. Thinking, what about using annotations for this? Basically we could
come up with something that looked like this:

LayerInfo {

@Proxy(target=MetadataLinkInfo.class)
List getMetadataLinks();

}

Which could then be recognized by ModProxy and an appropriate collection
wrapper wrapping individual elements in a proxy could be implemented.

That would work for MetadataLinkInfo, however, I don’t believe it
would be applicable
to VirtualTable and the INSPIRE module UniqueResourceIdentifiers, which are both
classes.
We cannot proxy them using ModificationProxy as far as I know…

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


Sorry, i meant to reply, not sure what happened.

Proceed as you see fit but i’ll suggest one more thing. We could use cglib to proxy for the classes. Afaik cglib is already a runtime dependency for GeoServer as its required by wicket, and i think spring. The proxy interface (MethodInterceptor) is of course different in cglib but it would be easy enough to implement one that backed on to InvocationHandler.

···

On Sat, May 25, 2013 at 4:00 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Final bump. This is a real problem breaking at least two modules, if I don’t get any further feedback I’ll go ahead with the initially proposed solution

Cheers

Andrea


Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

On Mon, May 6, 2013 at 10:38 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

Bump :wink:

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


On Tue, Apr 30, 2013 at 5:49 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Apr 30, 2013 at 5:26 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

The library is also quite small. The idea is that we’d use a Spring
registered Cloner object to deep clone the contents of all collections
before returning them out of a modification proxy.

Hmmm… i am kind of hesitant to add a new library dependency at such a core
level. Thinking, what about using annotations for this? Basically we could
come up with something that looked like this:

LayerInfo {

@Proxy(target=MetadataLinkInfo.class)
List getMetadataLinks();

}

Which could then be recognized by ModProxy and an appropriate collection
wrapper wrapping individual elements in a proxy could be implemented.

That would work for MetadataLinkInfo, however, I don’t believe it
would be applicable
to VirtualTable and the INSPIRE module UniqueResourceIdentifiers, which are both
classes.
We cannot proxy them using ModificationProxy as far as I know…

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


On Mon, May 27, 2013 at 9:45 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

Sorry, i meant to reply, not sure what happened.

Proceed as you see fit but i'll suggest one more thing. We could use cglib
to proxy for the classes. Afaik cglib is already a runtime dependency for
GeoServer as its required by wicket, and i think spring. The proxy
interface (MethodInterceptor) is of course different in cglib but it would
be easy enough to implement one that backed on to InvocationHandler.

Looked into it, it would work for the simple beans that we have now in the
map, but it is going to break if anything has final methods, since CGLIB
works by creating a subclass that overrides the parent methods. The cloning
library does not seem to be affected by these issues.

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

Do we have any beans with final methods that change state? My thought would be no since the classes we are talking about are typically dumb data projects. But i could be wrong.

···

On Tue, May 28, 2013 at 2:21 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Mon, May 27, 2013 at 9:45 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:


Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Sorry, i meant to reply, not sure what happened.

Proceed as you see fit but i’ll suggest one more thing. We could use cglib to proxy for the classes. Afaik cglib is already a runtime dependency for GeoServer as its required by wicket, and i think spring. The proxy interface (MethodInterceptor) is of course different in cglib but it would be easy enough to implement one that backed on to InvocationHandler.

Looked into it, it would work for the simple beans that we have now in the map, but it is going to break if anything has final methods, since CGLIB works by creating a subclass that overrides the parent methods. The cloning library does not seem to be affected by these issues.

Cheers

Andrea

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


On Tue, May 28, 2013 at 2:55 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

Do we have any beans with final methods that change state? My thought
would be no since the classes we are talking about are typically dumb data
projects. But i could be wrong.

We don't have any today, yet, the metadata map is a generic mechanism.
Or we can go the cglib way and update the metadata map javadoc demanding no
final methods in the objects stored there: if we add this new limitation it
has to be documented.

Thinking about it... the metadata map already requires that the objects be
put in there are Serializable,
so maybe the easiest thing to do is to deep clone them by serialization
(maybe using XStream?) and
be done with it.

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

On Tue, May 28, 2013 at 6:58 AM, Andrea Aime
<andrea.aime@anonymised.com>wrote:

On Tue, May 28, 2013 at 2:55 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

Do we have any beans with final methods that change state? My thought
would be no since the classes we are talking about are typically dumb data
projects. But i could be wrong.

We don't have any today, yet, the metadata map is a generic mechanism.
Or we can go the cglib way and update the metadata map javadoc demanding
no final methods in the objects stored there: if we add this new limitation
it has to be documented.

Agreed, and i don't think it would be too much of a limitation.

Thinking about it... the metadata map already requires that the objects be
put in there are Serializable,
so maybe the easiest thing to do is to deep clone them by serialization
(maybe using XStream?) and
be done with it.

True, if this works then it i am all for it.

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

On Tue, May 28, 2013 at 3:03 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

Thinking about it... the metadata map already requires that the objects be
put in there are Serializable,

so maybe the easiest thing to do is to deep clone them by serialization
(maybe using XStream?) and
be done with it.

True, if this works then it i am all for it.

I went down this path, trying to keep cloning to the bare minimum.

The wrap/clone logic is here, adopted several strategies to keep the
overhead low and try to wrap
what we know is wrappable using just ModificationProxy, and to copy by
clone() or
copy constructor when possible (also added copy constructors to a couple of
objects that I saw were
being copied over and over). Also for the copy by serialization I tried to
use binary serialization
(which gives a true copy) when possible, falling back on XStream only as a
last resort:
https://github.com/geoserver/geoserver/pull/245/files#L4R46

Changes to ModificationProxy are minimal, although I'm not sure about the
shallow/deep cloning
choices, in this case, deep for the collection that's returned, shallow for
the one that's kept as the original state:
https://github.com/geoserver/geoserver/pull/245/files#L3L106

In the process I've found a couple of things in WFS that basically relied
on the collections being modified
directly, and just kept a permanent reference to the config objects:
https://github.com/geoserver/geoserver/pull/245/files#L7L76
https://github.com/geoserver/geoserver/pull/245/files#L8R254

With these changes I can no longer reproduce the GUI editing issues (and
oh, added a test to cover
the unwanted change to metadata links contents).

I'd like to commit this on trunk. On 2.3.x, don't know....
For sure several bits of the GUI are broken with the current situation, but
I'm not sure what side
effects might happen by switching to wrapping/cloning. As the patch shows,
some bits of code
existed that relied on that bug, those are fixed, but there might be
others...
Screwed if you do, screwed if you don't kind of thing. Given that I
reported the issue and users
have not complained about it yet, I'd keep it on trunk only.
What do you think?

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

Very nice. The patch looks good to me. +1 for trunk.

For 2.3.x, not sure. I would certainly say let’s let the patch sit on trunk for a bit and if no known issues after a month or so then backport.

···

On Sun, Jun 2, 2013 at 3:31 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, May 28, 2013 at 3:03 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:


Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Thinking about it… the metadata map already requires that the objects be put in there are Serializable,

I went down this path, trying to keep cloning to the bare minimum.

The wrap/clone logic is here, adopted several strategies to keep the overhead low and try to wrap
what we know is wrappable using just ModificationProxy, and to copy by clone() or
copy constructor when possible (also added copy constructors to a couple of objects that I saw were
being copied over and over). Also for the copy by serialization I tried to use binary serialization
(which gives a true copy) when possible, falling back on XStream only as a last resort:
https://github.com/geoserver/geoserver/pull/245/files#L4R46

Changes to ModificationProxy are minimal, although I’m not sure about the shallow/deep cloning
choices, in this case, deep for the collection that’s returned, shallow for the one that’s kept as the original state:
https://github.com/geoserver/geoserver/pull/245/files#L3L106

In the process I’ve found a couple of things in WFS that basically relied on the collections being modified
directly, and just kept a permanent reference to the config objects:
https://github.com/geoserver/geoserver/pull/245/files#L7L76

https://github.com/geoserver/geoserver/pull/245/files#L8R254

With these changes I can no longer reproduce the GUI editing issues (and oh, added a test to cover
the unwanted change to metadata links contents).

I’d like to commit this on trunk. On 2.3.x, don’t know…
For sure several bits of the GUI are broken with the current situation, but I’m not sure what side
effects might happen by switching to wrapping/cloning. As the patch shows, some bits of code
existed that relied on that bug, those are fixed, but there might be others…
Screwed if you do, screwed if you don’t kind of thing. Given that I reported the issue and users
have not complained about it yet, I’d keep it on trunk only.
What do you think?

Cheers

Andrea

==
GeoServer training in Milan, 6th & 7th June 2013! Visit http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


True, if this works then it i am all for it.

so maybe the easiest thing to do is to deep clone them by serialization (maybe using XStream?) and
be done with it.

On Mon, Jun 3, 2013 at 3:29 PM, Justin Deoliveira <jdeolive@anonymised.com>wrote:

Very nice. The patch looks good to me. +1 for trunk.

For 2.3.x, not sure. I would certainly say let's let the patch sit on
trunk for a bit and if no known issues after a month or so then backport.

Yep, sounds like a nice middle ground. Ben, what do you say? :slight_smile:

Cheers
Andrea

--

GeoServer training in Milan, 6th & 7th June 2013! Visit
http://geoserver.geo-solutions.it for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

On 03/06/13 21:34, Andrea Aime wrote:

On Mon, Jun 3, 2013 at 3:29 PM, Justin Deoliveira <jdeolive@anonymised.com
<mailto:jdeolive@anonymised.com>> wrote:
    Very nice. The patch looks good to me. +1 for trunk.
    For 2.3.x, not sure. I would certainly say let's let the patch sit
    on trunk for a bit and if no known issues after a month or so then
    backport.
Yep, sounds like a nice middle ground. Ben, what do you say? :slight_smile:

:slight_smile:

--
Ben Caradoc-Davies <Ben.Caradoc-Davies@anonymised.com>
Software Engineer
CSIRO Earth Science and Resource Engineering
Australian Resources Research Centre