Skip to content
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

issue with specifying settings for gen_ #47

Closed
mforde84 opened this issue Feb 3, 2022 · 5 comments · Fixed by #54
Closed

issue with specifying settings for gen_ #47

mforde84 opened this issue Feb 3, 2022 · 5 comments · Fixed by #54
Assignees
Milestone

Comments

@mforde84
Copy link

mforde84 commented Feb 3, 2022

We're running into an issue with gen_smtp thats bouncing emails with max length greater than 10mb.

Gen_smtp controls this setting here:
https://github.com/gen-smtp/gen_smtp/blob/master/src/gen_smtp_server_session.erl#L71

However I'm not sure if its configurable in the rabbitmq_email server config block namely because the server_session.erl does not list it as an option:
https://github.com/gen-smtp/gen_smtp/blob/master/src/gen_smtp_server_session.erl#L93

Accompanying gen_smtp documentation seems to confirm this:

gen_smtp_server:start(
    smtp_server_example,
    [{sessionoptions, [{allow_bare_newlines, fix},
                       {callbackoptions, [{parse, true}]}]}]).
...
{sessionoptions, gen_smtp_server_session:options()} - see below
...
Session options are:

    {allow_bare_newlines, false | ignore | fix | strip} - see above
    {hostname, inet:hostname()} - which hostname server should send in response to HELO / EHLO commands. Default: inet:gethostname().
    {tls_options, [ssl:server_option()]} - options to pass to ssl:handshake/3 (OTP-21+) / ssl:ssl_accept/3 when STARTTLS command is sent by the client. Only needed if STARTTLS extension is enabled
    {protocol, smtp | lmtp} - when lmtp is passed, the control flow of the [Local Mail Transfer Protocol](https://tools.ietf.org/html/rfc2033) is applied. LMTP is derived from SMTP with just a few variations and is used by standard [Mail Transfer Agents (MTA)](https://en.wikipedia.org/wiki/Message_transfer_agent), like Postfix, Exim and OpenSMTPD to send incoming email to local mail-handling applications that usually don't have a delivery queue. The default value of this option is smtp.
    {callbackoptions, any()} - value will be passed as 4th argument to callback module's init/4

Am I missing something obvious here where it's possible to override this setting:
https://github.com/gen-smtp/gen_smtp/blob/fe4f164353d1a08fe0a64200c75fa358abe57b29/src/gen_smtp_server_session.erl#L37

I've tried through the rabbitmq_email server_config block but unsuccessfully

	{rabbitmq_email, [
		{server_config, [
			[{port, 2525}, {protocol, tcp}, {domain, "cqvlhbqe001.test.cm.com"},{auth, never}, {tls, never}, {address,{0,0,0,0}}], [{sessionoptions, [{maxsize, 200}]}]

I assume I need to merge a change to gen_smtp_server_session.erl to include maxsize as a definable option, correct? that or i need to fork, and build rabbitmq_email with my fork?

@mforde84
Copy link
Author

mforde84 commented Feb 3, 2022

ill attempt building with a fork and see what happens

@a-marcellini
Copy link

a-marcellini commented Oct 11, 2022

Hi,
I've stumbled upon the same problem then I planned to fork the gen_smtp and set another value for the max size.
However, when I tried to build the project, without any modification it didn't work.
It worked until 8/April, for sure, but not It doesn't build anymore.
The strange thing is that we haven't changed anything since then.
The error we get is the following:

#13 34.48 make[1]: Entering directory '/root/rabbitmq-email/.erlang.mk/rabbitmq-server/deps/rabbit_common'
#13 34.50  DEP    rabbitmq_codegen (master)
#13 34.51 error: no valid pathspec among:  master master master
#13 34.51 make[1]: *** [../../erlang.mk:5102: /root/rabbitmq-email/deps/rabbitmq_codegen] Error 1
#13 34.51 make[1]: Leaving directory '/root/rabbitmq-email/.erlang.mk/rabbitmq-server/deps/rabbit_common'
#13 34.51 make: *** [erlang.mk:8082: /root/rabbitmq-email/.erlang.mk/recursive-deps-list.log] Error 2

What we see is that the symbolic links to the modules are not created, but we ignore the reason.

Maybe there has been a change on the rabbitmq side?
Could you try to build the project just to see if it is a general problem or just mine?

Here you can find our try: sistemi-iungo#1

Thank you!

Repository owner deleted a comment from mforde84 Oct 11, 2022
@lukebakken
Copy link
Collaborator

@a-marcellini - please open a separate issue for the build problem. Most likely I need to update some files in this repo for recent RabbitMQ build changes.

@a-marcellini
Copy link

@a-marcellini - please open a separate issue for the build problem. Most likely I need to update some files in this repo for recent RabbitMQ build changes.

Done, Issue #52

@lukebakken lukebakken added this to the 1.1.0 milestone Oct 14, 2022
@lukebakken lukebakken self-assigned this Oct 14, 2022
@lukebakken
Copy link
Collaborator

We have to return a new max message size in the handle_HELO callback, as shown here:

https://github.com/gen-smtp/gen_smtp/blob/master/src/smtp_server_example.erl#L82-L86

lukebakken added a commit that referenced this issue Oct 14, 2022
Fixes #47

Bump gen_smtp to 1.2.0
lukebakken added a commit that referenced this issue Oct 17, 2022
Fixes #47

Bump gen_smtp to 1.2.0

Add new `server_maxsize` setting, allowing users to override 10MiB limit

Set maximum message size for EHLO as well, thanks @arjan

Since we depend on the latest RabbitMQ source, we cannot test using Erlang 23 or 24 as only 25 is currently supported

SIZE value must be a string

Update versions in README
lukebakken added a commit that referenced this issue Oct 17, 2022
Fixes #47

Bump gen_smtp to 1.2.0

Add new `server_maxsize` setting, allowing users to override 10MiB limit

Set maximum message size for EHLO as well, thanks @arjan

Since we depend on the latest RabbitMQ source, we cannot test using Erlang 23 or 24 as only 25 is currently supported

SIZE value must be a string

Update versions in README
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 a pull request may close this issue.

3 participants