| 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:40 |
| wgrant | Sorry, been distracted the last couple of days... | 08:41 |
| wgrant | That MP looks better now, thanks. | 08:41 |
| cjwatson | OK, thanks. | 08:42 |
| 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:52 |
| wgrant | Hm. | 08:53 |
| cjwatson | I mean, it's not actually breaking AFAICS, just surprising. | 08:53 |
| wgrant | Yeah | 08:54 |
| wgrant | I guess it falls afoul of the IP address restrictions because it has none. | 08:54 |
| 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:56 |
| 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:57 |
| 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. | 08:58 |
| 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:01 |
| 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:02 |
| cjwatson | Anyway, qa-ok since that's a pre-existing bug | 09:03 |
| cjwatson | Shall I file a bug? | 09:03 |
| wgrant | Please do. | 09:04 |
| wgrant | cjwatson: https://code.launchpad.net/~wgrant/launchpad/bug-1489674/+merge/269480 | 09:05 |
| 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:08 |
| 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:09 |
| 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:10 |
| 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:12 |
| 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:16 |
| cjwatson | Can you think of any reason not to plaster rSP over all recipient attribute accesses in mailers? | 09:17 |
| cjwatson | With BaseMailer, it's always a separate mail to each recipient so there should be no leakage. | 09:18 |
| wgrant | Indeed. | 09:22 |
| wgrant | Ideally you could do it higher. | 09:22 |
| cjwatson | Maybe in get_recipients? | 09:28 |
| wgrant | That might not help with eg. the rationale calculation in TeamMembershipMailer. | 09:31 |
| cjwatson | Which bit of it? | 09:32 |
| 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:33 |
| cjwatson | forExpiringMembership does admin.unique_displayname, if that's what you're thinking of | 09:34 |
| 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:35 |
| wgrant | Eeh. | 09:36 |
| wgrant | Indeed, probably safe. | 09:36 |
| cjwatson | It's true that forExpiringMembership would need its own handling as well. | 09:36 |
| 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:37 |
| cjwatson | (Is this really a possible setup?) | 09:38 |
| === mup_ is now known as mup | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!