cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: Findbugs report on 4.3-forward
Date Thu, 30 Jan 2014 13:28:30 GMT
Guys,

I found two reported issues of category 'scary'. To satify Rajuri's
concerns I would like to revert Ian's commit and checkin two changes
that change returning
byte[].toString()
into
Arrays.toString(byte[])
on return statements of generatePassword methods.

if no objections come in within a few...,
Daan

On Thu, Jan 30, 2014 at 1:53 PM, Daan Hoogland <daan.hoogland@gmail.com> wrote:
> Animesh, Ian,
>
> Can you comment on this?
>
> I couldn't find any findbugs issues of the the scariest kind in
> yesterdays version of the 4.3 branch. What was solved that needs to go
> in in spite of Rajuri's reservations?
>
> thanks,
> Daan
>
> On Thu, Jan 30, 2014 at 11:21 AM, Daan Hoogland <daan.hoogland@gmail.com> wrote:
>> Sorry Rajani,
>>
>> I had seen no reaction to Ian's explenation  and the request by
>> Animesh to pull it so I just did. let me look into it for a minute
>>
>> On Thu, Jan 30, 2014 at 5:36 AM, Rajani Karuturi
>> <Rajani.Karuturi@citrix.com> wrote:
>>> I see that the commit 9776e1af1c92486f5081b1ee8fa95cf0edb86b97 is already pushed
to 4.3. I don't see any response on my concern as well.
>>> Is it just me or anyone else sees a security issue with the generate password
change?
>>> Ian/Animesh/Daan, can you please respond?
>>>
>>> Thanks,
>>> ~Rajani
>>>
>>>
>>>
>>> On 29-Jan-2014, at 10:59 am, Rajani Karuturi <Rajani.Karuturi@citrix.com>
wrote:
>>>
>>>> Hi Ian,
>>>> Before it is pushed to 4.3, can you fix the generate password change like
i suggested in the other mail? This current change would make it less secure.
>>>>
>>>> Thanks,
>>>> ~Rajani
>>>>
>>>>
>>>>
>>>> On 29-Jan-2014, at 8:03 am, Ian Duffy <ian@ianduffy.ie> wrote:
>>>>
>>>>> Hi Animesh,
>>>>>
>>>>> Tested all those changes to detail. Those lines were removed due to
>>>>> unexpected behavior that I had not spotted until now.
>>>>>
>>>>> They were suppose to allow for better fall over between multiple domain
>>>>> controllers, how ever they were causing caching to occur. This meant
if a
>>>>> users password was reset in LDAP the old password was still allowing
login
>>>>> for a limited time.
>>>>>
>>>>> Please pull the changes forward,
>>>>> Thanks
>>>>>
>>>>> Ian.
>>>>> On 29 Jan 2014 00:07, "Animesh Chaturvedi" <animesh.chaturvedi@citrix.com>
>>>>> wrote:
>>>>>
>>>>>> If I look at this commit for example
>>>>>>
>>>>>>
>>>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commitdiff;h=92b4f66d73562e4211d2d787554ff229dbeb5705
>>>>>>
>>>>>> It removes the two lines from LdapContextFactory.java
>>>>>>
>>>>>> environment.put("com.sun.jndi.ldap.read.timeout", "500");-
>>>>>> environment.put("com.sun.jndi.ldap.connect.pool", "true");
>>>>>>
>>>>>> Is that reported by find bug? I don't know this code  so not sure
if it is
>>>>>> intentional or not ?
>>>>>>
>>>>>> The point is there may be unintended risks in allowing late changes.
>>>>>>
>>>>>>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Animesh Chaturvedi [mailto:animesh.chaturvedi@citrix.com]
>>>>>> Sent: Tuesday, January 28, 2014 3:35 PM
>>>>>> To: dev@cloudstack.apache.org
>>>>>> Subject: RE: Findbugs report on 4.3-forward
>>>>>>
>>>>>> Are you sure all of the ones are needed. A quick look at 20+ commits
from
>>>>>> Daan  show many formatting changes that may not be necessary and
hinder
>>>>>> quick review.
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Hugo Trippaers [mailto:trippie@gmail.com]
>>>>>> Sent: Tuesday, January 28, 2014 3:16 PM
>>>>>> To: dev@cloudstack.apache.org
>>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>>>
>>>>>>
>>>>>>
>>>>>> Sent from my iPhone
>>>>>>
>>>>>>> On 28 jan. 2014, at 23:50, Animesh Chaturvedi <
>>>>>> animesh.chaturvedi@citrix.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Hugo Trippaers [mailto:trippie@gmail.com]
>>>>>>> Sent: Tuesday, January 28, 2014 2:37 PM
>>>>>>> To: dev@cloudstack.apache.org
>>>>>>> Cc: dev@cloudstack.apache.org
>>>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>>>>
>>>>>>> Hey Animesh,
>>>>>>>
>>>>>>> Not in agreement here. These are squashed bugs and we want as
less bugs
>>>>>> in the release as possible.
>>>>>>> [Animesh] I understand but once we enter RC phase we only limit
>>>>>> important fixes. I have pulled in  2 commits from yours and 1 from
Daan.
>>>>>>
>>>>>> We limited our fixes to only the important issues that we found.
The other
>>>>>> 6000 issues between coverity and findbugs are still being triaged
and will
>>>>>> probably not make it into this release.
>>>>>>
>>>>>>>
>>>>>>> This is why we test any RC before we release it.
>>>>>>> [Animesh] Of course but timing is a bit off, if this was done
a month
>>>>>> back it would have been fine.
>>>>>>> I say include all the big fixes we have in the release. If that
means
>>>>>> more testing before we cut the RC then that is what it is. I can't
>>>>>> rightfully vote for a release with known issues with existing fixes.
>>>>>>> [Animesh] Any release will have known issues, if we have fixes
but can't
>>>>>> be sure on regression impact then we have to make a choice.
>>>>>>
>>>>>> Agreed, but we just don't agree on what that choice should be yet
;-)
>>>>>>
>>>>>>> Quality over release schedule would be my vote then.
>>>>>>> [Animesh] But why so late? Why was this activity not planned
early on? I
>>>>>> have been reminding community to call out issues early on since like
>>>>>> mid-December.
>>>>>>
>>>>>> On November 4 I sent the mail to the dev list that static code analysis
>>>>>> (coverity) found 6000+ issues that needed to be triaged. I worked
on quite
>>>>>> a few with my colleagues, but it's a big task for just the four of
us.
>>>>>> Findbugs just helped us to quickly identify the real scary issues
among
>>>>>> them.
>>>>>> So I agree that the timing is less than ideal, but we should do our
utmost
>>>>>> best to ship the highest quality piece of software we can.
>>>>>>
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Hugo
>>>>>>>
>>>>>>> Sent from my iPhone
>>>>>>>
>>>>>>>> On 28 jan. 2014, at 18:48, Animesh Chaturvedi <
>>>>>> animesh.chaturvedi@citrix.com> wrote:
>>>>>>>>
>>>>>>>> Folks these issues reported by find-bugs have existed for
some time. I
>>>>>> am not confident in picking them up now for 4.3 as it may break code
that
>>>>>> assumed old way of working. We can take them up for 4.3 maintenance
>>>>>> release. I wish we had done this exercise and not waited until now.
>>>>>>>>
>>>>>>>> I will pick Hugo's commit for which he called -1.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Animesh
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Trippie [mailto:trippie@gmail.com] On Behalf Of Hugo
Trippaers
>>>>>>>> Sent: Tuesday, January 28, 2014 1:29 AM
>>>>>>>> To: dev
>>>>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>>>>>
>>>>>>>> Hey Animesh,
>>>>>>>>
>>>>>>>> I agree with Daan here. We focussed on the bugs with a findbugs
>>>>>> annotation of scariest. I think that would warrant them to be included
in
>>>>>> the 4.3 release, so please cherry-pick them all.
>>>>>>>>
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>>
>>>>>>>> Hugo
>>>>>>>>
>>>>>>>>> On 28 jan. 2014, at 09:32, Daan Hoogland <daan.hoogland@gmail.com>
>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Animesh,
>>>>>>>>>
>>>>>>>>> I took up a lot of messages from findbugs in the server
package over
>>>>>>>>> the weekend. Not that I will attach my soul to the shipping
of my
>>>>>>>>> fixes but some of them are == vs eq and some are really
nasty
>>>>>>>>> nullpointer issues (a chack after first use is very common).
You can
>>>>>>>>> cherry-pick them or not. I don't think you should leave
any of them
>>>>>>>>> behind. Even with David being right we should go against
him at
>>>>>>>>> every convenient time, running the risk of being called
his wife.
>>>>>>>>>
>>>>>>>>>> On Tue, Jan 28, 2014 at 6:25 AM, Ian Duffy <ian@ianduffy.ie>
wrote:
>>>>>>>>>> Hi Animesh,
>>>>>>>>>>
>>>>>>>>>> Can you cherry-pick the below commit from from 4.3-forward
to 4.3
>>>>>> branch?
>>>>>>>>>>
>>>>>>>>>> Fix findbug issues within LDAP authenticator commit
>>>>>>>>>> 92b4f66d73562e4211d2d787554ff229dbeb5705
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Ian
>>>>>>>>>>
>>>>>>>>>> On 28 January 2014 03:48, Animesh Chaturvedi
>>>>>>>>>> <animesh.chaturvedi@citrix.com>wrote:
>>>>>>>>>>
>>>>>>>>>>> Hugo I was reviewing your commits to 4.3-forward
and looked at
>>>>>>>>>>> your commits
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;
>>>>>>>>>>> h = f18c5a1910b6370585a1d61638b8310c3ecba5ef
>>>>>>>>>>>
>>>>>>>>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;
>>>>>>>>>>> h = 60ac12780bfa1604902a89d5dc7937a8b9334e0d
>>>>>>>>>>> I think you want the last one which has fixes
for NetUtils and
>>>>>>>>>>> XenServerStorageMotionStrategy for which you
had put -1 in first
>>>>>>>>>>> RC but the commit includes more files. Can you
make limited
>>>>>>>>>>> changes directly to 4.3? I want to build another
RC later tonight
>>>>>>>>>>>
>>>>>>>>>>> Animesh
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: Animesh Chaturvedi [mailto:animesh.chaturvedi@citrix.com]
>>>>>>>>>>> Sent: Monday, January 27, 2014 1:30 PM
>>>>>>>>>>> To: dev@cloudstack.apache.org
>>>>>>>>>>> Subject: RE: Findbugs report on 4.3-forward
>>>>>>>>>>>
>>>>>>>>>>> Agreed
>>>>>>>>>>>
>>>>>>>>>>> We need to fix the most important ones for 4.3.
There may be
>>>>>>>>>>> assumptions in the code which we may not know
and may get broken
>>>>>>>>>>> if these issues are fixed late. I will pull in
the one Hugo casted
>>>>>>>>>>> his
>>>>>>>>>>> -1 for the first vote, any others?
>>>>>>>>>>>
>>>>>>>>>>> Animesh
>>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: David Nalley [mailto:david@gnsa.us]
>>>>>>>>>>> Sent: Monday, January 27, 2014 11:46 AM
>>>>>>>>>>> To: dev@cloudstack.apache.org
>>>>>>>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>>>>>>>>
>>>>>>>>>>> So just curious if I am the only one concerned
about a ton of
>>>>>>>>>>> fixes going in at the last minute. If the fixes
are for serious
>>>>>>>>>>> bugs and we have consensus around their severity
being high enough,
>>>>>> indeed lets fix things.
>>>>>>>>>>> My concern is that much of the QA we do is manual;
and while we
>>>>>>>>>>> are getting better; fixing tons of things at
the last minute may
>>>>>>>>>>> have unintended consequences that we don't know
about and won't
>>>>>> easily find.
>>>>>>>>>>>
>>>>>>>>>>> I yearn for the day when our automated testing
is broad enough
>>>>>>>>>>> that we can do fixes right up to the wire and
know that things
>>>>>>>>>>> still work, I am just not sure that I have confidence
that we are
>>>>>> there yet.
>>>>>>>>>>> Thoughts? I am being paranoid?
>>>>>>>>>>>
>>>>>>>>>>> --David
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Jan 27, 2014 at 3:11 AM, Daan Hoogland
>>>>>>>>>>> <daan.hoogland@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>> Animesh, I commented the once i made yesterday
with findbugs:
>>>>>>>>>>>>
>>>>>>>>>>>> I allready send a few and will get you a
list of the rest later
>>>>>> today.
>>>>>>>>>>>>
>>>>>>>>>>>> regards,
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, Jan 27, 2014 at 3:48 AM, Animesh
Chaturvedi
>>>>>>>>>>>> <animesh.chaturvedi@citrix.com> wrote:
>>>>>>>>>>>>> Good job fellas. I see a number of commits
20+ into 4.3-forward
>>>>>> branch.
>>>>>>>>>>> Are their specific commits you want me to pick
up out of these?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Animesh
>>>>>>>>>>>>>
>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>>>>>>>>>>>>> Sent: Sunday, January 26, 2014 2:41 AM
>>>>>>>>>>>>> To: dev
>>>>>>>>>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>>>>>>>>>>
>>>>>>>>>>>>> I didn't get very far last night and
will be looking at the
>>>>>>>>>>>>> server
>>>>>>>>>>> package again this afternoon.
>>>>>>>>>>>>>
>>>>>>>>>>>>> bon app├ętit,
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Sun, Jan 26, 2014 at 1:36 AM,
Ian Duffy <ian@ianduffy.ie>
>>>>>> wrote:
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Fixed the issues highlighted in the
ldap user authentication
>>>>>> package.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Have pushed to 4.3-forward.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Ian
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 25 January 2014 22:26, Daan Hoogland
>>>>>>>>>>>>>> <daan.hoogland@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> or reply to this mail with
the filename you are working on
>>>>>>>>>>>>>>> I'll be looking at the server
package as it seems to contain
>>>>>>>>>>>>>>> the most issues.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Sat, Jan 25, 2014 at 4:00
PM, Hugo Trippaers
>>>>>>>>>>>>>>> <hugo@trippaers.nl>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>> I've also added a job to
master with the Findbugs report and
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>> cobertura code coverage report.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Good stuff, we have a 12%
coverage of our classes with unit
>>>>>> tests.
>>>>>>>>>>>>>>>> Huge
>>>>>>>>>>>>>>> improvement over the last release
where we had 4% iirc. We
>>>>>>>>>>>>>>> have
>>>>>>>>>>>>>>> 306 reports from Findbugs, of
which the majority are
>>>>>>>>>>>>>>> internationalization
>>>>>>>>>>> issues.
>>>>>>>>>>>>>>> (String.getBytes without charset
mostly). On the coverity site
>>>>>>>>>>>>>>> we have
>>>>>>>>>>>>>>> 6000+ issues still open, but
at least that number is
>>>>>>>>>>>>>>> 6000+ relatively stable, we
>>>>>>>>>>>>>>> fix as much issues as we introduce
and it's untuned so we can
>>>>>>>>>>>>>>> assume a large number of false
positives there.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I think that on average the
automated tools tell us that code
>>>>>>>>>>>>>>>> quality is
>>>>>>>>>>>>>>> improving, which a good thing.
Combined with the functional
>>>>>>>>>>>>>>> testing and the simulator build
we can prove that we are doing
>>>>>>>>>>>>>>> quite well on the code quality
angle.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> http://jenkins.buildacloud.org/job/build-master-slowbuild/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hugo
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 25 jan. 2014, at 14:13,
Daan Hoogland
>>>>>>>>>>>>>>>> <daan.hoogland@gmail.com>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> H Hugo,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I'll spend some time
on it tonight. Do you have a work load
>>>>>>>>>>>>>>>>> distribution scheme or
is it random access?
>>>>>>>>>>>>>>>>> ;)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> regards
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Sat, Jan 25, 2014
at 12:39 PM, Hugo Trippaers
>>>>>>>>>>>>>>>>> <hugo@trippaers.nl>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>> Hey all,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I've made Jenkins
run the findbugs analysis on 4.3-forward.
>>>>>>>>>>>>>>>>>> Is there
>>>>>>>>>>>>>>> somebody who is willing to help
triage the findings? Maybe
>>>>>>>>>>>>>>> there is some stuff that we need
to fix?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> the url is
>>>>>>>>>>>>>>> http://jenkins.buildacloud.org/job/cloudstack-4.3-forward-mave
>>>>>>>>>>>>>>> n
>>>>>>>>>>>>>>> -
>>>>>>>>>>>>>>> bui
>>>>>>>>>>>>>>> ld
>>>>>>>>>>>>>>> /3/findbugsResult/
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hugo
>>>>>>>>
>>>>>>
>>>>
>>>

Mime
View raw message