[00:21] abentley: https://code.launchpad.net/~abentley/launchpad/duplicate-msgid/+merge/62697 seems a bit heavy-handed. [00:22] abentley: I can understand skipping duplicate messages to the same place... but not to all of Launchpad. [00:22] abentley: (that includes mailing lists, so you can no longer send an email both to a bug and a mailing list) [00:23] huh [00:23] the preview diff is empty :( [00:23] r13139 [00:28] hmm [00:28] I agree [00:28] duplicate message ids are normal across the system [00:29] only abnormal in one context - same list, or same bug etc [00:29] Exactly. [00:29] Shall I roll it back? [00:29] its 7:30 pm for Aaron [00:29] yes, I think it needs rollback [00:29] we can't deploy with it [00:30] but [00:30] lets check just a little more [00:30] Sure. [00:31] so the error encountered is a duplicate msgid on one bug [00:31] when the notification for it is being raised [00:31] Well, adding the duplicate message results in a DB exception. [00:31] bug 595166 [00:32] <_mup_> Bug #595166: IntegrityError raised filing a bug using the email interface

< https://launchpad.net/bugs/595166 > [00:32] That's the one. [00:32] its raised when *notifying* tha the second message was added [00:32] Only because that's when the flush occurs. [00:32] Oh. [00:33] no, *really* when notifying [00:33] That's not the OOPS I"ve seen before. [00:33] The one I saw violated bugmessage__bug__message__key [00:33] thats on a link to message [00:33] not to two messages with the same msgid [00:34] Hmm? [00:34] OTOH so is the notification one [00:34] Two emails with same msgid and same content => one Message row [00:34] sure [00:34] this check isn't complete though [00:34] which is the other issue [00:38] aieee [00:38] other broken code [00:38] for email_addr in addresses: [00:38] Oh? [00:38] discards all but the last handler found [00:38] Heh [00:38] see the end of handle_one_mail [00:39] looks like using cc: can make our mail handling do some interesting and odd stuff [00:39] anyhoo [00:39] *blinkers* [00:39] Why would it be using To or Cc? [00:40] It should be using X-Launchpad-To [00:40] shoo. Go read. [00:40] Which is the envelope sender. [00:40] thou shalt be blind. And don't say I didn't warn thee. [00:40] So, yes, it uses X-Launchpad-To unless it's not present. [00:40] Not that bad. [00:41] well [00:41] we've some funky in our mail pipeline and we haven't identified it [00:41] Well, we fixed some of it two weeks ago. [00:54] ok please rollback [00:54] I have written up a hopefully helpful explanation for abentley [00:55] if you've other or better suggestions for how to tackle it, please say so [00:59] lifeless: Done, thanks/ [01:01] thank you [01:14] Project windmill-db-devel build #347: STILL FAILING in 1 hr 5 min: https://lpci.wedontsleep.org/job/windmill-db-devel/347/ [01:50] lifeless: The security hole is this: using a crafted or predicted msgid an attacker can cause Launchpad to generate a link to a mail that was sent to a private bug or mailing list when they have no such permission. [01:50] lifeless: I thought so too. [01:50] But it's not the case. [01:50] I investigated when I looked at trying to fix this bug a few weeks back. [01:51] It grabs all the Messages with the right Message-ID, and compares the content. [01:51] You can possibly obtain attachments if you know all the textual content, I guess. === Ursinha-afk is now known as Ursinha [01:52] wgrant: fromMessage is fine [01:52] wgrant: aarons patch wasn't [01:55] lifeless: I don't see a new security bug. The returned message isn't used for anything. It just returns None to exit the handler, right? [01:57] wgrant: if the patch were changed to call the handlers using the result of messageset.get(rfc822id), it would open a security hole. [01:57] lifeless: Yes. [01:57] (not to mention still crashing in the same way) [01:57] wgrant: thats what I meant. [01:58] the usage of messageset.get(rfc822id) is not sufficient to determine 'same email' nor 'can see the email' [01:59] Ah, it helps if I read your entire reply. [01:59] *snort* [02:00] It was long, and my initial skim revealed a statement that I knew to be false! [02:00] feel free to reply and editorialise [02:24] Project windmill-devel build #155: STILL FAILING in 1 hr 10 min: https://lpci.wedontsleep.org/job/windmill-devel/155/ [02:28] hi all [02:31] hi poolie [02:31] poolie: random question for you [02:32] if the lp bzr+ssh/sftp codehosting project were a separate tree, do you think your team would do anything interesting with it ? [02:37] lifeless: hmm. Occasionally users ask about how to run a bzr server with some access control and/or authentication that isn't easily satisfied by using authorized_keys directives and filesystem permissions [02:38] lifeless: so I guess having a less intimidating tree to extract that from, or at least learn from, might be helpful to someone. [02:39] hi lifeless [02:39] spiv: tim has extracted the code but adapted it to a uni hosting solution [02:39] well [02:39] so one of the candidate services - and I think a low hanging fruit - is one that still talks xmlrpc [02:39] you know there was john's forking lp server project [02:39] that would probably have needed to touch only that code [02:39] and landing it has been pretty hard [02:40] partly just because of bugs that appeared under heavy load; and those would have needed to be solved anyhow [02:40] right [02:40] but, i think it would certainly have made it easier [02:40] I was going to say [02:40] for instance easier to run up an ec2 instance and hammer it [02:40] poolie: really? [02:40] 'ec2 demo'; wait; hammer it. [02:40] hm [02:41] i wonder if jam tried that [02:41] perhaps not actually any easier [02:41] spivs comment is more interesting :) [02:41] anyhow i guess there are two questions here [02:41] the friction-to-change-in-lp aspect is well covered I think [02:41] :-P [02:41] I'm curious about other interesting things [02:42] i don't think this would really directly satisfy those users [02:42] like - adopt it for bzr's on server (make some of the policies pluggable, for instance) [02:42] it might be a step towards having a common codebase [02:42] *own* [02:43] hm, there is an _apparent_ contradiction here with wanting to make codebrowse more folded into launcphad [02:43] how so ? [02:43] actually [02:43] at any rate there is a kind of precedent there for having one program that is used both by lp and externally [02:43] contractors here [02:43] I will be back in a bit [02:43] ah no [02:43] false alarum [02:45] a few dimensions: [02:45] - would this make it easier to make changes on codehosting in lp.net [02:45] - are we actually likely to want to make any changes to it [02:45] - would it make it easier to reuse externally [02:47] so cool interesting things is actually what I wanted to ask about :) [02:47] you could say no to all your three things there and still do something cool or interesting with it [02:48] I merely mean to encourage out-of-box stuff [02:48] it could well do [02:48] hm [02:51] could well do [02:52] i may be inside the box but it seems like they would still be gated by needing to actually get run somewhere [02:52] either deployed on lp or run by other people [02:53] sure [02:53] perhaps saying to to /all 3/ isn't plausible :) [02:54] at the moment i feel those 3 dimensions all get a weak yes at the moment [02:54] i'm trying to think of things that would make sense to do in there but not in standalone bzr serve [02:54] perhaps structured audit logs or notifications? [02:58] it implements ssh [02:58] so user management [02:58] multiplexing onto one account [02:59] mm [02:59] oh, easy deployment on windows? [02:59] fsvo easy :) assuming you have to install twisted and everything [02:59] brb [03:00] it would be an interesting case where you'd like to provide a fake/simple implementation of the other side of the services it uses [03:00] the registry type things read by xmlrpc [03:25] yes [04:12] wow [04:12] only 2 failures [04:18] lifeless: On? [04:22] mailed you for your edification [04:23] Ah, that bug. [04:23] hmm [04:23] person.join(team) and team.addMember are asymmetric [04:23] I suspect this will cause issues [04:24] however [04:24] I shall close my eyes and go bravely forward. [04:25] (specifically team.join doesn't honour admin or owner rights) [04:25] it prefilters, wrongly [04:28] hah [04:28] it goes deeper though [04:28] getDirectAdministrators includes the owner [04:28] I'll leave that for now [04:28] The bug says that. [04:31] bah so it does [04:33] actually [04:33] I'm going to exclude that [04:34] there is a (reasonable) argument that owner implies admin [04:34] (or that our admin stuff is not granular enough) [04:34] mmm, but then it might be confusing. [04:35] it would be odd for an owner to have to join the team to rename it or change its metadata [04:35] so - excluding admin for now [04:48] why does the sample data have someone called 'dumper' ? >< [04:49] Same reason as mark is there, I guess. [04:49] Renamed to avoid pings. [04:49] ah [04:56] wgrant: do you have any ideas about the first failure there ? [04:56] its got me a little flummoxed [05:00] mark has a row for membership in ~admins [05:02] hah [05:02] I wonder if we have naffed sample data [05:02] mark.inTeam(getUtility(ILaunchpadCelebrities).admin) [05:02] False [05:03] Project windmill-db-devel build #348: STILL FAILING in 1 hr 5 min: https://lpci.wedontsleep.org/job/windmill-db-devel/348/ [05:13] lifeless: Hah. Probably. [05:13] lifeless: There's a script for that. [05:19] its more subtle than that [05:19] can_edit_team is returning False [05:20] also its checking the way-bong side of the equation but thats a different discussion [05:20] Hm, TP seems consistent. [05:20] :( [05:23] eah [05:24] s/^/y [05:29] ok its true at the start [05:29] something in this test script corrupts it [05:30] bsearch time [05:31] and looks like we manually expire mark from admins [05:31] Yay [05:31] line 612 [05:32] this test was only working because mark owns ~admins in the sample data [05:32] Awesome. [05:33] which is what I'm changing [05:37] ok now its just weird [05:37] admin_person *is* an admin [05:37] but its still throwing a security proxy error [05:42] inTeam cache busted? [05:42] I tried disabling that [05:42] its not reach inTeam [05:42] so a cached security assertion or something [05:54] lifeless: is there a pattern we use to bulk load (eager fetch) fields of type CollectionField? rather than just single object references [05:55] yes, see lp.services.database.bulk.load_referenced [05:55] thanks [05:57] lifeless: you mean load_referencing? i'm using that and it's not preventing db hits [05:57] You need to make it a cachedproperty. [05:58] wallyworld_: that gets the data, it won't seed it for traversals. [05:58] bugger [05:59] it's not worht making it a cachedproperty since the field is only referenced once for each object [05:59] i guess there's no way to bulk load *and* seed the traversals? [06:01] why do you say its not worth it ? [06:01] also which types are we talking about ? [06:02] wgrant: >< its still using 'mark' for some -bizarre- reason [06:02] * lifeless tries to figure it out [06:04] lifeless: there's a list of 6 people. we display irc nick. we only have to access the irc nic collection field once per rendering per person [06:04] but that does one query per person [06:04] right, you want a cached property. [06:04] totally appropriate [06:04] see the other use of load_referencing [06:06] in this case, the irc nic is only defined as a class field of type SQLMultipleJoin. so i will need to introduce a new property? [06:06] yes [06:07] you need to move the join attribute to a private attribute [06:07] ok [06:08] add a cached property with the same name as the current join attribute [06:08] seed it appropriately and deal with fallout [06:08] Do you need the join attribute? [06:09] well, perhaps not. [06:09] Just Stormify it. [06:09] In the cachedproperty. [06:09] Return a list of a Stormified query. [06:09] wgrant: Reference is storm :P [06:09] Less SQLObject, better for the world :) [06:09] SQLMultipleJoin is not. [06:09] true [06:10] but stormify != remove join attribute [06:10] anyhow [06:10] wtf [06:10] pasting you a snippet [06:10] k, I am intrigued. [06:10] also - Person.preferredemail is a cachedproperty. but it hits the database once per person cause it does a query inside the property getter. and no luck so far seeding the data [06:10] lifeless: Hm? I meant create a Stormified query. [06:10] Not Stormify the class. [06:10] wallyworld_: Which name are you seeding? [06:10] wgrant: yes, you could also just use a storm Reference directly. [06:11] ReferenceSet, possibly, yes. [06:11] Hmm. [06:11] How is admin_person set there? [06:11] perhaps i'm not doing the seeding correctly [06:11] bulk.load_referencing(EmailAddress, persons, ['personID']) [06:11] wgrant: >>> admin_person = personset.getByEmail(ADMIN_EMAIL) [06:11] wallyworld_: use PersonSet.getPrecachedPersonsByIds [06:11] it does the correct sql from what i can see, but.... [06:12] wallyworld_: and listify its output [06:12] right. will take a look. thanks [06:12] wallyworld_: *always* use that when dealing with persons [06:12] wallyworld_: also change the vocab to just return the Person ids [06:12] lifeless: Shouldn't that always be name16? [06:12] wgrant: yes. [06:12] wgrant: and it is! [06:12] So why is the test saying it should be something else? [06:13] exactly! [06:13] lifeless: so this will need to be done across the board for all person related vocabs [06:13] wallyworld_: one query at a time [06:13] Welcome to massive layering violations :) [06:13] np, just wondering [06:14] i guess the vocab stuff predated the getPrecachedPersons... stuff [06:14] yes [06:14] until 11 months ago we liked to requery everything [06:15] right. [06:16] context = makes more sense [06:28] can has review? https://code.launchpad.net/~lifeless/launchpad/bug-227494/+merge/62943 [06:31] lifeless: Have you checked the teams that this will affect? [06:31] They were mostly Ubuntu teams when I last looked. [06:31] ~admins :) [06:32] no I haven't [06:34] lifeless: putting a commit message on my restfulclient mp didn't cause it to land [06:35] 1300 rows [06:35] poolie: hmm, need to ask someone that knows. benji or gary come to mind. [06:35] poolie: unless README says something useful [06:35] "cause it to land"? [06:35] Landings don't happen automatically. [06:36] i guess what i really want to ask is: how is the trunk of restfulclient maintained [06:36] by pqm, tarmac, manually, etc [06:36] i could mail them [06:36] wgrant: some subprojects use tarmac [06:36] lazr.restful is manual. [06:36] Not sure about restfulclient. [06:37] lifeless: Do they? [06:37] i'll subscribe benji [06:37] thanks [06:37] lifeless: person.merged IS NOT NULL [06:37] Er, IS NULL [06:37] Cuts it down to 500ish. [06:37] wgrant: 471 [06:37] wgrant: already done that [06:38] wgrant: some are truely noddy - https://launchpad.net/~ubuntu-jewish [06:38] lifeless: You're calling *that* noddy? [06:38] There are far worse :) [06:38] Far, far worse. [06:38] wgrant: I am - it has 1 proposed member, and the owner has left the team. [06:38] wgrant: I wasn't saying anything about the name or intent of the team. [06:39] I know. [06:39] But there are the teams which invite every team. [06:39] And have no members. [06:39] yes [06:39] affected by this ? [06:39] Like the one that showed up last week. [06:39] Invited dozens of teams. [06:39] Owner had left it. [06:39] Awesome. [06:39] win [06:39] anyhow [06:39] so, I think we want to do this [06:40] And then some of the invited teams had bad admin settings, so randoms approved them, and the real admins couldn't leave :( [06:40] Yes. [06:40] I think so. [06:40] I will happily blog before it goes live [06:40] its arguably exploitable today [06:41] (find something that checks one way but not the other and leverage it) [06:41] Yeah [07:00] poolie: it should still be approved ;) [07:00] poolie: I just meant ask benji about landing mechanisms [07:02] ffs "ConfigParser.NoOptionError: No option 'consumer_key' in section: '1'" _again_ [07:02] do you ever see this? [07:10] should we subscribe some lp developer team to loggerhead reviews? [07:11] at the moment they seem to go by default to.. beuno and ~bzr or something [07:12] loggerhead-reviewers is the reviewer team [07:12] https://launchpad.net/~loggerhead-reviewers/+members [07:12] so they already are [07:12] oh, ok [07:12] i must have implicitly claimed it [07:13] nm then [07:16] Project windmill-db-devel build #349: STILL FAILING in 1 hr 6 min: https://lpci.wedontsleep.org/job/windmill-db-devel/349/ === almaisan-away is now known as al-maisan === al-maisan is now known as almaisan-away [07:47] wgrant: follow up on https://code.launchpad.net/~lifeless/launchpad/bug-227494/+merge/62943 ? [07:49] lifeless: You are arbitrarily changing the user to an admin half-way through because the owner has no privs. [07:49] That may be invalidating subsequent tests. [07:50] Even if it isn't, it's rather confusing to change it partially globally when only that one block needs it. [07:50] wgrant: have you looked through that doctest ? [07:50] hint: it does this a lot already [07:50] It's a doctest. No. [07:50] and the previous user was one it thought was an admin. [07:50] Ah, I see. [07:50] OK. [07:50] An admin, but not an Admin. [07:50] Not an ~admin, sorry. [07:50] yes, a ~admin [07:51] How did it stop being an admin? [07:51] because it was mark [07:51] Because it was the owner of ~admins? [07:51] yes [07:51] But the test removed that membership? [07:51] yes [07:51] ahhhhhhhh [07:51] Carry on, then. [07:51] it was BROKEN [07:51] Preferably beating that test to death on the way. [07:53] now I just need a reviewer. [07:53] Perhaps we should graduate you now [07:53] Hi StevenK. [07:53] Heh [07:53] Possibly. [07:53] I have been waiting for you to consistently look below the surface on non-soyuz code [07:54] which you have been improving on [07:54] I don't recall finding things you missed recently other than the extraordinarily messy gpghandler stuff, which confused everyone. [07:55] so, I think graduation is in order [07:56] Yeah, the gpghandler example is bad, but I don't think anyone non-Twisted and non-you would have caught that either.' [07:56] Since most people don't have much idea of Twisted :( [07:58] facepalm [07:58] * lifeless sobs [07:58] ? [07:58] The other test? [07:58] wgrant: are you being mean to lifeless again??? [07:58] https://code.launchpad.net/~xaav/loggerhead/export-tarball/+merge/62941 [07:58] copy and paste from bzrlib [07:58] with mild tweaks [07:59] Heh [07:59] lifeless: So, am I graduating? [07:59] Good morning [08:00] wgrant: yes, I am mailing -dev [08:00] lifeless: Thanks! [08:00] wgrant: Grats! [08:03] return dumps({ [08:03] 'row_id': 'tasksummary%s' % self.context.id, [08:03] 'bugtask_path': '/'.join( [08:03] [''] + canonical_url(self.context).split('/')[3:]), [08:05] lifeless: BugSummary.viewed_by is painful because of indirect subscriptions. [08:06] stub: they don't grant visibility [08:06]