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.hoogl...@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.hoogl...@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.karut...@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.karut...@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 <i...@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.chaturv...@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.chaturv...@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:trip...@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.chaturv...@citrix.com> wrote: >>>>>>> >>>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Hugo Trippaers [mailto:trip...@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.chaturv...@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:trip...@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.hoogl...@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 <i...@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.chaturv...@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.chaturv...@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:da...@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.hoogl...@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.chaturv...@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.hoogl...@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 <i...@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.hoogl...@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 >>>>>>>>>>>>>>> <h...@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.hoogl...@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 >>>>>>>>>>>>>>>>> <h...@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 >>>>>>>> >>>>>> >>>> >>>
