Re: [exim] server_set_id no longer working?

On 2012-11-06 at 10:32 +0200, James Isolder wrote:
> I have tried it for both the new code and both still don't output the
> set_id. I am freebsd 9 if that helps at all.

Ah, no, my mistake. Sorry. :(

I thought the auth vars should only be set if authentication succeeded.
In fact, they *can* be set even if authentication fails, and it's the
return code which matters.

The expanded value of `server_set_id` goes into a short-lived variable,
which is used for logging and diagnostics; _only_ if the authenticator
also returned `OK` does this value make it into the `$authenticated_id`
C-level variable.

I must have known this at some point, because the authenticators I've
written myself do the right thing. *scratches head* Dementia, that
must be it.

Okay, so new plan:
----------------------------8< cut here >8------------------------------
201 auth_vars[0] = expand_nstring[1] = msgbuf;
202 expand_nlength[1] = Ustrlen(msgbuf);
203 expand_nmax = 1;
204
205 /* clean up globals which aren't referenced, but still shouldn't be left
206 pointing to stack memory */
207 #define CLEANUP_RETURN(Code) do { auth_vars[0] = expand_nstring[1] = NULL; \
208 expand_nlength[1] = expand_nmax = 0; return (Code); } while (0);
----------------------------8< cut here >8------------------------------

Ugh, that trailing semi-colon on line 208 was also dumb.

Change line 201 to:
----------------------------8< cut here >8------------------------------
auth_vars[0] = expand_nstring[1] = string_copy(msgbuf);
----------------------------8< cut here >8------------------------------

Change lines 207,208 to:
----------------------------8< cut here >8------------------------------
#define CLEANUP_RETURN(Code) do { return (Code); } while (0)
----------------------------8< cut here >8------------------------------

If that works, then the commit will be to rip out the CLEANUP_RETURN()
calls entirely and use "return" directly, but changing the macro is a
simpler change for testing.

(You can make this change with or without the change I previously
requested; worst case scenario is that when auth succeeds, a few more
bytes are allocated from a pool, to hold the userid twice).

-Phil