You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martijn Dashorst <ma...@gmail.com> on 2015/11/09 10:34:20 UTC

WICKET-6021: add commons-collections as a dependency or not...

One of the problems we ran into while fixing 6021 was the ability to
use a Linked[Hash]Map. The default JDK version doesn't have any public
or protected API to give us the previous and next entries. We need
those to retrieve the next element when removing a child from a markup
container.

Unfortunately the JDK collections don't have any extensibility that
would allow us to graft those missing methods upon the existing
collections.

Commons Collections provides a LinkedMap class that does give us those
methods. Unfortunately this forces us to add a core dependency, or we
should copy the specific code into our project.

Adding a dependency is bad because it adds more stuff to track--not
just for us, but for our users as well--, provides additional
headaches (as you may have noticed, there's an deserialization issue
with commons-collections).

Moving the code from com-col to Wicket is also bad, as we take on the
maintenance burden, the code in question takes about 30-ish classes to
copy, and we duplicate code that is available from elsewhere
(duplication is bad mkay)

Emond's suggestion is to move the code, strip it of all that we don't
need and maintain that ourselves. I'd like to add that we should make
that code package private or name it such that it doesn't conflict
with com-col on a class name base.

But before we go on that path, we'd like to hear if folks have better ideas?

Martijn

Re: WICKET-6021: add commons-collections as a dependency or not...

Posted by Tobias Soloschenko <to...@googlemail.com>.
Oh! I'm sorry :-/ - I was not following the whole discussion - just read about collections and saw this article. 

kind regards

Tobias

> Am 10.11.2015 um 23:31 schrieb Martin Grigorov <mg...@apache.org>:
> 
> You watch out :p
> Martijn mentioned it in his mail:  (as you may have noticed, there's an
> deserialization issue with commons-collections).
> And here is the answer:
> https://blogs.apache.org/foundation/entry/apache_commons_statement_to_widespread
> 
> 
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
> 
> On Tue, Nov 10, 2015 at 11:18 PM, Tobias Soloschenko <
> tobiassoloschenko@googlemail.com> wrote:
> 
>> Hi,
>> 
>> please watch out - there is a security issue mentioned at heise.de (zero
>> day exploid) which affects common collections:
>> 
>> org/apache/commons/collections/functors/InvokerTransformer.class
>> 
>> Here is an article in english which mentions that issue:
>> 
>> http://www.infoq.com/news/2015/11/commons-exploit
>> 
>> kind regards
>> 
>> Tobias
>> 
>>> Am 10.11.2015 um 22:37 schrieb Martin Grigorov <mg...@apache.org>:
>>> 
>>> +1 to keep the dependency
>>> 
>>> I've also took a look at the code and it seems we will need to copy quite
>>> some classes.
>>> It seems the introduction of commons-fileupload and commons-io as
>>> dependencies in 7.0.0 didn't lead to any complains so far.
>>> 
>>> Martin Grigorov
>>> Wicket Training and Consulting
>>> https://twitter.com/mtgrigorov
>>> 
>>> On Mon, Nov 9, 2015 at 10:34 AM, Martijn Dashorst <
>>> martijn.dashorst@gmail.com> wrote:
>>> 
>>>> One of the problems we ran into while fixing 6021 was the ability to
>>>> use a Linked[Hash]Map. The default JDK version doesn't have any public
>>>> or protected API to give us the previous and next entries. We need
>>>> those to retrieve the next element when removing a child from a markup
>>>> container.
>>>> 
>>>> Unfortunately the JDK collections don't have any extensibility that
>>>> would allow us to graft those missing methods upon the existing
>>>> collections.
>>>> 
>>>> Commons Collections provides a LinkedMap class that does give us those
>>>> methods. Unfortunately this forces us to add a core dependency, or we
>>>> should copy the specific code into our project.
>>>> 
>>>> Adding a dependency is bad because it adds more stuff to track--not
>>>> just for us, but for our users as well--, provides additional
>>>> headaches (as you may have noticed, there's an deserialization issue
>>>> with commons-collections).
>>>> 
>>>> Moving the code from com-col to Wicket is also bad, as we take on the
>>>> maintenance burden, the code in question takes about 30-ish classes to
>>>> copy, and we duplicate code that is available from elsewhere
>>>> (duplication is bad mkay)
>>>> 
>>>> Emond's suggestion is to move the code, strip it of all that we don't
>>>> need and maintain that ourselves. I'd like to add that we should make
>>>> that code package private or name it such that it doesn't conflict
>>>> with com-col on a class name base.
>>>> 
>>>> But before we go on that path, we'd like to hear if folks have better
>>>> ideas?
>>>> 
>>>> Martijn
>> 

Re: WICKET-6021: add commons-collections as a dependency or not...

Posted by Martin Grigorov <mg...@apache.org>.
You watch out :p
Martijn mentioned it in his mail:  (as you may have noticed, there's an
deserialization issue with commons-collections).
And here is the answer:
https://blogs.apache.org/foundation/entry/apache_commons_statement_to_widespread


Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov

On Tue, Nov 10, 2015 at 11:18 PM, Tobias Soloschenko <
tobiassoloschenko@googlemail.com> wrote:

> Hi,
>
> please watch out - there is a security issue mentioned at heise.de (zero
> day exploid) which affects common collections:
>
> org/apache/commons/collections/functors/InvokerTransformer.class
>
> Here is an article in english which mentions that issue:
>
> http://www.infoq.com/news/2015/11/commons-exploit
>
> kind regards
>
> Tobias
>
> > Am 10.11.2015 um 22:37 schrieb Martin Grigorov <mg...@apache.org>:
> >
> > +1 to keep the dependency
> >
> > I've also took a look at the code and it seems we will need to copy quite
> > some classes.
> > It seems the introduction of commons-fileupload and commons-io as
> > dependencies in 7.0.0 didn't lead to any complains so far.
> >
> > Martin Grigorov
> > Wicket Training and Consulting
> > https://twitter.com/mtgrigorov
> >
> > On Mon, Nov 9, 2015 at 10:34 AM, Martijn Dashorst <
> > martijn.dashorst@gmail.com> wrote:
> >
> >> One of the problems we ran into while fixing 6021 was the ability to
> >> use a Linked[Hash]Map. The default JDK version doesn't have any public
> >> or protected API to give us the previous and next entries. We need
> >> those to retrieve the next element when removing a child from a markup
> >> container.
> >>
> >> Unfortunately the JDK collections don't have any extensibility that
> >> would allow us to graft those missing methods upon the existing
> >> collections.
> >>
> >> Commons Collections provides a LinkedMap class that does give us those
> >> methods. Unfortunately this forces us to add a core dependency, or we
> >> should copy the specific code into our project.
> >>
> >> Adding a dependency is bad because it adds more stuff to track--not
> >> just for us, but for our users as well--, provides additional
> >> headaches (as you may have noticed, there's an deserialization issue
> >> with commons-collections).
> >>
> >> Moving the code from com-col to Wicket is also bad, as we take on the
> >> maintenance burden, the code in question takes about 30-ish classes to
> >> copy, and we duplicate code that is available from elsewhere
> >> (duplication is bad mkay)
> >>
> >> Emond's suggestion is to move the code, strip it of all that we don't
> >> need and maintain that ourselves. I'd like to add that we should make
> >> that code package private or name it such that it doesn't conflict
> >> with com-col on a class name base.
> >>
> >> But before we go on that path, we'd like to hear if folks have better
> >> ideas?
> >>
> >> Martijn
> >>
>

Re: WICKET-6021: add commons-collections as a dependency or not...

Posted by Tobias Soloschenko <to...@googlemail.com>.
Hi,

please watch out - there is a security issue mentioned at heise.de (zero day exploid) which affects common collections:

org/apache/commons/collections/functors/InvokerTransformer.class

Here is an article in english which mentions that issue:

http://www.infoq.com/news/2015/11/commons-exploit

kind regards

Tobias

> Am 10.11.2015 um 22:37 schrieb Martin Grigorov <mg...@apache.org>:
> 
> +1 to keep the dependency
> 
> I've also took a look at the code and it seems we will need to copy quite
> some classes.
> It seems the introduction of commons-fileupload and commons-io as
> dependencies in 7.0.0 didn't lead to any complains so far.
> 
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
> 
> On Mon, Nov 9, 2015 at 10:34 AM, Martijn Dashorst <
> martijn.dashorst@gmail.com> wrote:
> 
>> One of the problems we ran into while fixing 6021 was the ability to
>> use a Linked[Hash]Map. The default JDK version doesn't have any public
>> or protected API to give us the previous and next entries. We need
>> those to retrieve the next element when removing a child from a markup
>> container.
>> 
>> Unfortunately the JDK collections don't have any extensibility that
>> would allow us to graft those missing methods upon the existing
>> collections.
>> 
>> Commons Collections provides a LinkedMap class that does give us those
>> methods. Unfortunately this forces us to add a core dependency, or we
>> should copy the specific code into our project.
>> 
>> Adding a dependency is bad because it adds more stuff to track--not
>> just for us, but for our users as well--, provides additional
>> headaches (as you may have noticed, there's an deserialization issue
>> with commons-collections).
>> 
>> Moving the code from com-col to Wicket is also bad, as we take on the
>> maintenance burden, the code in question takes about 30-ish classes to
>> copy, and we duplicate code that is available from elsewhere
>> (duplication is bad mkay)
>> 
>> Emond's suggestion is to move the code, strip it of all that we don't
>> need and maintain that ourselves. I'd like to add that we should make
>> that code package private or name it such that it doesn't conflict
>> with com-col on a class name base.
>> 
>> But before we go on that path, we'd like to hear if folks have better
>> ideas?
>> 
>> Martijn
>> 

Re: WICKET-6021: add commons-collections as a dependency or not...

Posted by Martin Grigorov <mg...@apache.org>.
+1 to keep the dependency

I've also took a look at the code and it seems we will need to copy quite
some classes.
It seems the introduction of commons-fileupload and commons-io as
dependencies in 7.0.0 didn't lead to any complains so far.

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov

On Mon, Nov 9, 2015 at 10:34 AM, Martijn Dashorst <
martijn.dashorst@gmail.com> wrote:

> One of the problems we ran into while fixing 6021 was the ability to
> use a Linked[Hash]Map. The default JDK version doesn't have any public
> or protected API to give us the previous and next entries. We need
> those to retrieve the next element when removing a child from a markup
> container.
>
> Unfortunately the JDK collections don't have any extensibility that
> would allow us to graft those missing methods upon the existing
> collections.
>
> Commons Collections provides a LinkedMap class that does give us those
> methods. Unfortunately this forces us to add a core dependency, or we
> should copy the specific code into our project.
>
> Adding a dependency is bad because it adds more stuff to track--not
> just for us, but for our users as well--, provides additional
> headaches (as you may have noticed, there's an deserialization issue
> with commons-collections).
>
> Moving the code from com-col to Wicket is also bad, as we take on the
> maintenance burden, the code in question takes about 30-ish classes to
> copy, and we duplicate code that is available from elsewhere
> (duplication is bad mkay)
>
> Emond's suggestion is to move the code, strip it of all that we don't
> need and maintain that ourselves. I'd like to add that we should make
> that code package private or name it such that it doesn't conflict
> with com-col on a class name base.
>
> But before we go on that path, we'd like to hear if folks have better
> ideas?
>
> Martijn
>

Re: WICKET-6021: add commons-collections as a dependency or not...

Posted by andrea del bene <an...@gmail.com>.
I also have these mixed feelings about this decision. I would prefer to 
go for the second solution (copy the needed code) but if we are talking 
about of copying dozens of classes then it makes more sense to add the 
dependency.

PS: just for curiosity, does Guava have something similar to offer?

On 09/11/2015 11:18, Sebastien wrote:
> Hi Martijn,
>
> About the deserialization issue, I suppose you make reference to this:
> http://news.softpedia.com/news/the-vulnerability-that-will-rock-the-entire-java-world-495840.shtml
>
> I agree on your entire point of view: adding a dependency is bad, having
> our own copied-classes is also bad. So is the dilemma, and I have no better
> idea than Emond...
>
> Just a concern: having our own copy has side effects in addition to the
> fact we should maintain it. For instance with another case, the JSONObject
> has been taken from json.org. It is boring me because I cannot use/transmit
> it to the business tier, whereas having used org.json.JSONObject would have
> been simpler to manage...
>
> So I am mixed between having (and maintaining) less code and having a
> dependency. But actually, apache commons are very convenient, widely used
> and almost a standard; so I would prefer this option short before "taking
> only what we need" one....
>
> My 2 cents :)
> Sebastien.
>
>
>
>
>
>
>
>
>
>
>
> On Mon, Nov 9, 2015 at 10:34 AM, Martijn Dashorst <
> martijn.dashorst@gmail.com> wrote:
>
>> One of the problems we ran into while fixing 6021 was the ability to
>> use a Linked[Hash]Map. The default JDK version doesn't have any public
>> or protected API to give us the previous and next entries. We need
>> those to retrieve the next element when removing a child from a markup
>> container.
>>
>> Unfortunately the JDK collections don't have any extensibility that
>> would allow us to graft those missing methods upon the existing
>> collections.
>>
>> Commons Collections provides a LinkedMap class that does give us those
>> methods. Unfortunately this forces us to add a core dependency, or we
>> should copy the specific code into our project.
>>
>> Adding a dependency is bad because it adds more stuff to track--not
>> just for us, but for our users as well--, provides additional
>> headaches (as you may have noticed, there's an deserialization issue
>> with commons-collections).
>>
>> Moving the code from com-col to Wicket is also bad, as we take on the
>> maintenance burden, the code in question takes about 30-ish classes to
>> copy, and we duplicate code that is available from elsewhere
>> (duplication is bad mkay)
>>
>> Emond's suggestion is to move the code, strip it of all that we don't
>> need and maintain that ourselves. I'd like to add that we should make
>> that code package private or name it such that it doesn't conflict
>> with com-col on a class name base.
>>
>> But before we go on that path, we'd like to hear if folks have better
>> ideas?
>>
>> Martijn
>>


Re: WICKET-6021: add commons-collections as a dependency or not...

Posted by Maxim Solodovnik <so...@gmail.com>.
I would also prefer to add dependency

On Mon, Nov 9, 2015 at 4:18 PM, Sebastien <se...@gmail.com> wrote:

> Hi Martijn,
>
> About the deserialization issue, I suppose you make reference to this:
>
> http://news.softpedia.com/news/the-vulnerability-that-will-rock-the-entire-java-world-495840.shtml
>
> I agree on your entire point of view: adding a dependency is bad, having
> our own copied-classes is also bad. So is the dilemma, and I have no better
> idea than Emond...
>
> Just a concern: having our own copy has side effects in addition to the
> fact we should maintain it. For instance with another case, the JSONObject
> has been taken from json.org. It is boring me because I cannot
> use/transmit
> it to the business tier, whereas having used org.json.JSONObject would have
> been simpler to manage...
>
> So I am mixed between having (and maintaining) less code and having a
> dependency. But actually, apache commons are very convenient, widely used
> and almost a standard; so I would prefer this option short before "taking
> only what we need" one....
>
> My 2 cents :)
> Sebastien.
>
>
>
>
>
>
>
>
>
>
>
> On Mon, Nov 9, 2015 at 10:34 AM, Martijn Dashorst <
> martijn.dashorst@gmail.com> wrote:
>
> > One of the problems we ran into while fixing 6021 was the ability to
> > use a Linked[Hash]Map. The default JDK version doesn't have any public
> > or protected API to give us the previous and next entries. We need
> > those to retrieve the next element when removing a child from a markup
> > container.
> >
> > Unfortunately the JDK collections don't have any extensibility that
> > would allow us to graft those missing methods upon the existing
> > collections.
> >
> > Commons Collections provides a LinkedMap class that does give us those
> > methods. Unfortunately this forces us to add a core dependency, or we
> > should copy the specific code into our project.
> >
> > Adding a dependency is bad because it adds more stuff to track--not
> > just for us, but for our users as well--, provides additional
> > headaches (as you may have noticed, there's an deserialization issue
> > with commons-collections).
> >
> > Moving the code from com-col to Wicket is also bad, as we take on the
> > maintenance burden, the code in question takes about 30-ish classes to
> > copy, and we duplicate code that is available from elsewhere
> > (duplication is bad mkay)
> >
> > Emond's suggestion is to move the code, strip it of all that we don't
> > need and maintain that ourselves. I'd like to add that we should make
> > that code package private or name it such that it doesn't conflict
> > with com-col on a class name base.
> >
> > But before we go on that path, we'd like to hear if folks have better
> > ideas?
> >
> > Martijn
> >
>



-- 
WBR
Maxim aka solomax

Re: WICKET-6021: add commons-collections as a dependency or not...

Posted by Sebastien <se...@gmail.com>.
Hi Martijn,

About the deserialization issue, I suppose you make reference to this:
http://news.softpedia.com/news/the-vulnerability-that-will-rock-the-entire-java-world-495840.shtml

I agree on your entire point of view: adding a dependency is bad, having
our own copied-classes is also bad. So is the dilemma, and I have no better
idea than Emond...

Just a concern: having our own copy has side effects in addition to the
fact we should maintain it. For instance with another case, the JSONObject
has been taken from json.org. It is boring me because I cannot use/transmit
it to the business tier, whereas having used org.json.JSONObject would have
been simpler to manage...

So I am mixed between having (and maintaining) less code and having a
dependency. But actually, apache commons are very convenient, widely used
and almost a standard; so I would prefer this option short before "taking
only what we need" one....

My 2 cents :)
Sebastien.











On Mon, Nov 9, 2015 at 10:34 AM, Martijn Dashorst <
martijn.dashorst@gmail.com> wrote:

> One of the problems we ran into while fixing 6021 was the ability to
> use a Linked[Hash]Map. The default JDK version doesn't have any public
> or protected API to give us the previous and next entries. We need
> those to retrieve the next element when removing a child from a markup
> container.
>
> Unfortunately the JDK collections don't have any extensibility that
> would allow us to graft those missing methods upon the existing
> collections.
>
> Commons Collections provides a LinkedMap class that does give us those
> methods. Unfortunately this forces us to add a core dependency, or we
> should copy the specific code into our project.
>
> Adding a dependency is bad because it adds more stuff to track--not
> just for us, but for our users as well--, provides additional
> headaches (as you may have noticed, there's an deserialization issue
> with commons-collections).
>
> Moving the code from com-col to Wicket is also bad, as we take on the
> maintenance burden, the code in question takes about 30-ish classes to
> copy, and we duplicate code that is available from elsewhere
> (duplication is bad mkay)
>
> Emond's suggestion is to move the code, strip it of all that we don't
> need and maintain that ourselves. I'd like to add that we should make
> that code package private or name it such that it doesn't conflict
> with com-col on a class name base.
>
> But before we go on that path, we'd like to hear if folks have better
> ideas?
>
> Martijn
>