cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ian Duffy <...@ianduffy.ie>
Subject Re: Findbugs report on 4.3-forward
Date Thu, 30 Jan 2014 19:50:36 GMT
Hi,

Sorry about the delay in replying to this have been doing exams at uni all
week.

Daan's change looks to change Rajani concern.

Might be me being naive but I fail to understand the concern fully...
The given character selection was roughly 72 with a string of length 20. If
my math is correct thats 72^20 different possible combinations...

Anyways, thanks for taking care of this Daan.

On 30 January 2014 13:28, Daan Hoogland <daan.hoogland@gmail.com> wrote:

> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message