>> +    if (in_stream == NULL) {
>> +        /* must fail the entire transaction */
>> +        chat_reset(state, var_smtpd_hist_thrsh);
>> +        mail_reset(state);
>> +        rcpt_reset(state);
>> +        return -1;
>> +    }
> 
> Why no response to the client?

The function imap_open() responds to the client before it returns NULL.
        in_stream = imap_open(state, url);

>> +    case SMTP_ERR_EOF:
>> +        smtpd_chat_reply(state, "554 4.6.6 EOF from IMAP server");
>> +        vstream_longjmp(state->client, SMTP_ERR_QUIET);
>> +        break;
> 
> Why is the DSN code 4.X.X when the SMTP reply code is 5XX? Is this a
> permanent or a transient error code? 

It is a transient failure.  The reasoning for these particular codes was as 
follows.  RFC 4468 section 3.2 states "If the URL fetch fails, the server will 
fail the entire transaction."  RFC 5321 section 4.2.2 uses code 554 for 
"Transaction failed."  And the table in RFC 5248 section 2.4 implies that a 
4.6.6 is valid with a 554.  If this interpretation of the RFCs is incorrect, 
please propose corrected response codes.


The remainder of your feedback speaks to style and to weaknesses in the 
implementation that I pointed out in the cover letter to the code contribution. 
 That cover letter also said:
Feel free to [...] restructure or rewrite the code as desired,
as long as you preserve our copyright.  We understand that our
implementation choices may differ from yours; if you see a better way to
achieve the same goal please do adopt the better way.

Reply via email to