Skip to content

Propagate the error to the client if the external command fails #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 25, 2022

Conversation

ca0s
Copy link
Contributor

@ca0s ca0s commented Jul 28, 2021

This change makes the external command code return a generic 554 error to the SMTP client if the external command fails, so it knows sending failed and can retry.

This change makes the external command code return a generic 554 error to the SMTP client if the external command fails, so it knows sending failed and can retry.
Make the return error code style coherent with the rest of the code. Make the error message sent to the SMTP client when the external command fails more precise.
@decke
Copy link
Owner

decke commented Jul 28, 2021

One last question is which SMTP status code to use in this case. 5xx codes will tell the client that the issue is permanent so do not try again. 451 sounds more appropriate and will indicate a temporary error.

@ca0s
Copy link
Contributor Author

ca0s commented Jul 29, 2021

I chose 514 because in my specific use case (my external command sends the email through an HTTP API) it is actually a permanent error: it only fails if the message body is too big.

I don't have any issues changing it to 451, as my clients will react the same.

Would you be OK if I sent another PR defining a list of status codes for external commands so smtprelay can choose the best SMTP status code? Something like:

command status code SMTP status code
0 250, mail sent
-1 451, temporary error, try again
-2 554, generic permanent error
-3 554 5.3.4, permanent error, message too big

Kind regards, and thank you for writing this tool.

@decke
Copy link
Owner

decke commented Jul 29, 2021

Instead of defining a list I'd prefer to leave this to the external command. So the external commands should return SMTP status codes as error codes. We won't be able to pass along nice error texts though.

@ca0s
Copy link
Contributor Author

ca0s commented Jul 29, 2021

That would be the cleanest solution. However, UNIX does not allow process return codes outside of the range [0,255]. Returning values out of that range yields unexpected codes in a standard Linux box:

javi@pwn ~/test/go $ cat return_fixed.go
package main

import "os"

func main() {
        os.Exit(514)
}
javi@pwn ~/test/go $ cat exec.go
package main

import (
        "fmt"
        "os/exec"
        "os"
)

func main() {
        cmd := exec.Command(os.Args[1])
        err := cmd.Run()
        exitcode := cmd.ProcessState.ExitCode()
        fmt.Printf("err: %s, exitcode: %d\n", err, exitcode)
}
javi@pwn ~/test/go $ ./exec ./return_fixed
err: exit status 2, exitcode: 2

Therefore, I'm afraid some mapping will be required.

@decke decke merged commit b134e42 into decke:master Apr 25, 2022
@decke
Copy link
Owner

decke commented Apr 25, 2022

Thanks a lot for your contribution!

@ca0s ca0s deleted the patch-1 branch April 25, 2022 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants