procmail
[Top] [All Lists]

Re: variable assignment from pipe, multiple lines

2002-01-14 16:10:42
First off, thank you for your very nice and detailed response,
there were many things to improve I was not aware of before.

On Sun, Jan 13, 2002 at 10:47:03PM -0600, David W. Tamkin wrote:
Stefan Alfredsson asked,

| :0 c:

And we're off.  OK, either you want the parent procmail to serialize runs
of the clones or not.  If you don't (and you probably don't), get rid of
the second colon; if you do, you have to put a name for the local lockfile
to the right of the second colon, because a left brace gives the parent
procmail nothing to infer a lockfile name from.

As I understood the man-page the lockfile name was optional and would be 
created automatically .. but reading it again I might have misunderstood.

Without the "$" modifier on that condition, $XLOOP means newline plus the
five letters "xloop" (matching is case-insensitive in the absence of the `D'
flag), so we need the "$" modifier to get variable substitution.  Also,
$\XLOOP is preferable, so that all characters in the value of $XLOOP will be
taken literally.

ahh... well its obvious in hindsight that $ would match a newline
with XLOOP on the next line .. that advice probably saved a lot of
headache and looping emails :) 


| {
|         :0 W

I have a feeling that get_vac doesn't read in any text from the message, so
procmail will report a write error if you don't have `i' on the recipe that
ends up running get_vac.

aha, I tried a solution like ( cat ; ... ) to avoid the error, but 'i'
is a better solution

| * .
Just omit it.  An unconditional recipe needs no conditions.

ok

|         VACMSG=| /sbin/get_vac ldapdb "ou=pop,dc=acme,dc=com" $LOGNAME

To see what's really happening, put

LOG="The value of the VACMSG variable is \"$VACMSG\"
" # closing quote is one line down

at that point in your rcfile and see what is logged.  You *are* setting a
logfile, aren't you?

yes, this reveals that everything infact is captured.

By the way, are you sure that "dc=" should be in there twice?  I don't know
from beans about get_vac, but that looks funny to me.  If you say it's right,
it's right.

Yes, this is the way the ldap directory is set up. The command works
when ran "standalone" as well, so nothing is wrong there.

Splitting the company domain name into multiple domainComponents (dc's)
is used for example in the openldap admin guide at
http://www.openldap.org/doc/admin/quickstart.html

|         :0 a   # If vacation was activated, send reply.

We're not going to need the body in the action of this recipe, so add an `h'
flag there to spare procmail the effort of feeding the body.

improving efficiency is always welcomed

|     # Rewrite from->to, to->from, append vacationmessage
|         | ( formail -R To: From: -i From: | formail -ri From \
|                 -R Old-From From -U From \

Oh my stars.  You just want to interchange To: and From: (a bad idea anyway,
since To: can have multiple entries, and you wouldn't want your new From: to
look like that)?  You can do it this way (but it's still a bad idea):

 formail -R To: From: -R From: To: -I "Precedence: bulk" -A"X-Loop: $XLOOP"

However, if your goal is to prepare a return header, just use formail -r and
let formail figure out the To: and From:.  It's very good at that.

Well, I used that approach when I first started looking at sending
an automatic reply. The problem however is that formail used 
userid(_at_)mailhost
as reply address.

Usually, the accounts exist in a sendmail virtusertable, meaning that mail
for foo(_at_)bar(_dot_)com is delivered to local account pop1234.

When I used -r to generate an autoreply, someone mailing foo(_at_)bar(_dot_)com
would get a reply back from pop1234(_at_)mailgw(_dot_)acme(_dot_)com, which is 
somewhat
confusing. So the idea was to just interchange from and to headers,
but I clearly see the problem when it is sent to multiple addresses.
One solution would be to "reverse lookup" from the accountid.
And even then, one account could have multiple aliases, so they
would need to be matched to the To: addresses.

(I just now did an experiment with -r, it seems no From: header
 is genereated, but formail leaves that to the MTA)


Also, you're missing the semicolon between the end of formail's options and
the echo command.  I'm surprised you're not getting a whole pile of errors.

my mistake; it was there earlier but then that line was cut
to improve clarity of my recipe when sending to the list.
Obviously it added confusion instead, sorry.

|                 echo $VACMSG \

OK, I'll bet that that is the cause of your problem.  Are you using csh or a
derivative of csh as procxmail's $SHELL?  Don't.  Assign SHELL=/bin/sh at the
top of the rcfile.  csh and csh-based shells are notorious for being unable
to deal with embedded newlines.  Set your shell to /bin/sh and change that
line to

The shell is set to /bin/sh (ie bash), because the pop-users do not
have a valid shell assigned (which procmail tries to use if $SHELL is not set)

 echo "$VACMSG" \

yes! this was the core of my problem; the newlines were expanded and
interpreted as commands instead of text.

Now I only need to make sure noone can enter quotation-marks into the
database, because of the problems it would create. Hmm.. maybe do a quick
char replacement in the get_vac program.. " -> ' .. but thats inefficient
because its done many times. better to do it once when the message is set
to save cycles.

I doubt that; I think that the whole text is in the variable but that 
echo is outputting only the first line.  Getting the variable logged by
procmail will tell us for sure.

yes, you were right, it was captured in full.

HOWEVER, there really is no need for forking a clone there, so let's ease the
load on the CPU and on the process table a little bit:

I just wanted to be sure to operate on a copy of the mail, so
it would always be delivered if my recipe would fail somehow

However, I found myself with two copies delivered instead. Will
remove the unnecessary copying.

Finally, unless somehow get_vac takes care of this, you really should main-
tain a cache of addresses who have already received that vacation period's
message from that user and not send tell anyone more than once.  

get_vac does not handle this, and it would be a good idea to
only send one reply once in a while.

 {
  CACHE=.vacationcache # in $MAILDIR by default, else use absolute path
  CACHESIZE=16384 # or whatever; powers of two are traditional

The original reason for not doing this was that the users did
not have any home directory; with all account info (alias/forward etc)
in the LDAP database, only the mailspool would be kept in the filesystem.

However, I just got the idea that all of them could be in one
directory instead.

I.e. CACHE=/var/spool/vac_cache/cache.$LOGNAME


Thank you again for your time and effort in this matter.

If you have any idea about how to solve the alias -> account
From: header, please share it as well :)

Regards,
 Stefan
_______________________________________________
procmail mailing list
procmail(_at_)lists(_dot_)RWTH-Aachen(_dot_)DE
http://MailMan.RWTH-Aachen.DE/mailman/listinfo/procmail

<Prev in Thread] Current Thread [Next in Thread>