* nupas auth: incompatibility and error messaging
@ 2017-03-22 0:02 inkswinc
2017-03-22 2:53 ` [9front] " Benjamin Purcell
0 siblings, 1 reply; 3+ messages in thread
From: inkswinc @ 2017-03-22 0:02 UTC (permalink / raw)
To: 9front
Nupas' smtp.c file uses a different var in the 'server=' part of the
string it queries from factotum than upas did: where upas used
'server=smtp.gmail.com' (from the dialstring passed on the
commandline), nupas uses 'server=gmail-smtp-msa.l.google.com' (result
of some lookup, I presume) Not only does this break backwards
compatibility, but it also feels a little more brittle to me; the
latter name seems likely to change.
Not sure where else this difference occurs; it doesn't seem to in
imaps, which I also use, but it does seem to in /sys/lib/tls/smtp
(though with gmail that needs to change all the time anyways, so...)
It also doesn't do a good job of error messaging when auth fails; it
just shows up as an error in the 'hello' phase, which sucks.
Following patch is an attempt at resolving those issues; it works for
me. This is the first time I've ever given anyone else any C code
I've written, so feel free to be ruthless if you see anything
suboptimal.
diff -r a01d0802d023 sys/src/cmd/upas/smtp/smtp.c
--- a/sys/src/cmd/upas/smtp/smtp.c Mon Mar 20 19:15:40 2017 +0100
+++ b/sys/src/cmd/upas/smtp/smtp.c Tue Mar 21 23:49:01 2017 +0000
@@ -493,20 +493,27 @@
static char *
doauth(char *methods)
{
- char *buf, *err;
+ char *buf, *err, *factstring;
UserPasswd *p;
int n;
DS ds;
- dialstringparse(ddomain, &ds);
+ dialstringparse(farend, &ds);
if(strstr(methods, "CRAM-MD5"))
return smtpcram(&ds);
- p = auth_getuserpasswd(nil,
- user?"proto=pass service=smtp server=%q user=%q":"proto=pass service=smtp server=%q",
+ factstring = smprint(user?
+ "proto=pass service=smtp server=%q user=%q":
+ "proto=pass service=smtp server=%q",
ds.host, user);
- if (p == nil)
+
+ p = auth_getuserpasswd(nil,factstring);
+ if (p == nil) {
+ syslog(0, "smtp.fail", "failed to get userpasswd for %s", factstring);
+ free(factstring);
return Giveup;
+ }
+ free(factstring);
err = Retry;
if (strstr(methods, "LOGIN")){
Thanks,
sam-d
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [9front] nupas auth: incompatibility and error messaging
2017-03-22 0:02 nupas auth: incompatibility and error messaging inkswinc
@ 2017-03-22 2:53 ` Benjamin Purcell
2017-03-22 2:58 ` Benjamin Purcell
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Purcell @ 2017-03-22 2:53 UTC (permalink / raw)
To: 9front
At the very least you should check that the char* returned from
smprint is not nil.
But if this were me I would avoid the memory allocation entirely since
we can estimate a reasonable bound on the auth string. In that case I
like to use a static buf instead and print into it sequentially. That
way you also avoid an arguably ugly ternary expression and avoid
another memory allocation that is found below:
http://okturing.com/src/1190/body
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [9front] nupas auth: incompatibility and error messaging
2017-03-22 2:53 ` [9front] " Benjamin Purcell
@ 2017-03-22 2:58 ` Benjamin Purcell
0 siblings, 0 replies; 3+ messages in thread
From: Benjamin Purcell @ 2017-03-22 2:58 UTC (permalink / raw)
To: 9front
If no one objects to the change sam-d wants to make with switching
ddomain to farend, I'll apply the patch I posted soon.
-spew
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-22 2:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-22 0:02 nupas auth: incompatibility and error messaging inkswinc
2017-03-22 2:53 ` [9front] " Benjamin Purcell
2017-03-22 2:58 ` Benjamin Purcell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).