[08:40] <cjwatson> wgrant: Does my amendment to https://code.launchpad.net/~cjwatson/launchpad/delay-ppa-publication/+merge/269090 look OK?
[08:40] <cjwatson> wgrant: Also, want me to do your pending QA?
[08:40] <wgrant> cjwatson: Ah, if you could.
[08:41] <wgrant> Sorry, been distracted the last couple of days...
[08:41] <wgrant> That MP looks better now, thanks.
[08:42] <cjwatson> OK, thanks.
[08:52] <cjwatson> wgrant: I seem to get "URL not allowed" surprisingly often
[08:52] <cjwatson> e.g. http://lies.example.com/ resulted in that
[08:52] <cjwatson> When I'd have expected "DNS lookup failed"
[08:53] <wgrant> Hm.
[08:53] <cjwatson> I mean, it's not actually breaking AFAICS, just surprising.
[08:54] <wgrant> Yeah
[08:54] <wgrant> I guess it falls afoul of the IP address restrictions because it has none.
[08:56] <cjwatson> Ah, could be.
[08:56] <wgrant> I'll need to refactor the rules to remove the positive dst assertion, I think.
[08:56] <cjwatson> http://www.ubuntu.com/ also gave me "URL not allowed" but that's more understandable.
[08:56] <wgrant> Which is possible if they get split up, I think.
[08:56] <wgrant> Right, that is intended.
[08:57] <wgrant> I expected the DNS error to happen before the ACL check, TBH.
[08:57] <cjwatson> Trying to think of a way to provoke a different error without actually having to bother setting up my own endpoint.
[08:58] <wgrant> You can get connection errors by using a host that doesn't listen on HTTPS.
[08:58] <wgrant> Or HTTP.
[08:58] <cjwatson> Oh that's true
[08:58] <wgrant> Has to be port 80
[08:58] <wgrant> And HTTPS is dodgy because of httplib, so 80 is better to test.
[09:01] <cjwatson> wgrant: https://oops.canonical.com/?oopsid=OOPS-86856e3812ab2bada21cf3f23ab8f83b
[09:01] <cjwatson> Though apparently not a regression.
[09:01] <wgrant> Hrmph.
[09:01] <wgrant> Did you provoke a timeout?
[09:02] <wgrant> It indeed shouldn't OOPS.
[09:02] <cjwatson> (That's from trying my ADSL, riva.dynamic.greenend.org.uk, which doesn't listen on HTTP but also apparently doesn't actually reject it)
[09:02] <wgrant> Ah.
[09:02] <wgrant> Right.
[09:02] <cjwatson> Useful misconfigurations FTW
[09:03] <cjwatson> Anyway, qa-ok since that's a pre-existing bug
[09:03] <cjwatson> Shall I file a bug?
[09:04] <wgrant> Please do.
[09:05] <wgrant> cjwatson: https://code.launchpad.net/~wgrant/launchpad/bug-1489674/+merge/269480
[09:08] <cjwatson> wgrant: https://bugs.launchpad.net/launchpad/+bug/1489792
[09:08] <mup> Bug #1489792: WebhookClient.deliver crashes if the endpoint times out <fallout> <oops> <Launchpad itself:Triaged> <https://launchpad.net/bugs/1489792>
[09:08] <wgrant> Thanks
[09:09] <cjwatson> wgrant: Mystifying, why is to_person ever a team there, let alone a private one?
[09:09] <wgrant> cjwatson: If the team has an email address.
[09:09] <cjwatson> Oh, right.
[09:09] <cjwatson> And we don't need this in BaseMailer too?
[09:10] <wgrant> BaseMailer already seems to touch other attributes without rSP
[09:10] <cjwatson> It does, but I wonder if it has hitherto only been called from scripts
[09:10] <wgrant> So I thought it must have done it globally somewhere else.
[09:10] <wgrant> That is possible indeed.
[09:12] <cjwatson> One of these days the word order in ZopelessDatabaseLayer vs. DatabaseFunctionalLayer will stop confusing me.  Or I'll crack and rename one of them.
[09:12] <wgrant> ZopelessDatabaseLayer needs to be renamed for two reason.
[09:12] <cjwatson> wgrant: Anyway, r=me but I should check whether any of my BaseMailer conversions are potentially affected.
[09:16] <cjwatson> I think team-mail for example can go wrong if you renew a team membership and one of the admins of the team is an inaccessible private team.
[09:16] <wgrant> Yep
[09:16] <cjwatson> get_recipients does rSP but BaseMailer doesn't.
[09:17] <cjwatson> Can you think of any reason not to plaster rSP over all recipient attribute accesses in mailers?
[09:18] <cjwatson> With BaseMailer, it's always a separate mail to each recipient so there should be no leakage.
[09:22] <wgrant> Indeed.
[09:22] <wgrant> Ideally you could do it higher.
[09:28] <cjwatson> Maybe in get_recipients?
[09:31] <wgrant> That might not help with eg. the rationale calculation in TeamMembershipMailer.
[09:32] <cjwatson> Which bit of it?
[09:33] <wgrant> It unusually grabbed the displayname directly for something.
[09:33] <wgrant> One of the templates had %%(blahblah)s because it was formatting the format stirng.
[09:34] <cjwatson> forExpiringMembership does admin.unique_displayname, if that's what you're thinking of
[09:35] <cjwatson> The stuff in TeamRecipientReason is fine because it always refers directly to the member or the team being operated on, rather than other members/admins of the same team; and when performing one of these operations you always have access to both the member and the team at hand.
[09:35] <cjwatson> team.teamowner is used, but that's constrained to be a public person.
[09:36] <wgrant> Eeh.
[09:36] <wgrant> Indeed, probably safe.
[09:36] <cjwatson> It's true that forExpiringMembership would need its own handling as well.
[09:37] <cjwatson> But it actually needs some special handling no matter what, because it tells you the names of the team's admins.  If you're in the bizarre situation of being a member of a team with some admins you aren't allowed to know about, the expiring membership mail shouldn't tell you
[09:38] <cjwatson> (Is this really a possible setup?)
=== mup_ is now known as mup