Content type params utf8

[kape1395]

No description provided.

[mworrell]

Looks ok to me, especially with the added test case. Wondering if the encode_paramater changes are affecting other headers.

@seriyps what do you think?

[seriyps]

I, honestly, don't remember how exactly this part works. Would be nice to get some comments with explanation from @kape1395. But, since no tests were broken, I think it's ok

[kape1395]

I tried to keep the other headers unchanged. As I understand, ContentTypeParams are only used to construct the ContentType and the ContentDisposition headers. Other headers are used as provided by the user, therefore the user is responsible for formatting their values properly.

[mworrell]

@kape1395 could you rebase this on the newest master?

[kape1395]

I have rebased my changes on the master. But this test is failing, maybe because it tries to access external server.

					{"Erlang 23.2.7 compatibility",
					 fun() ->
							 Receipt = gen_smtp_client:send_blocking({<<"Info.ContactTracing@sg.ch">>, [<<"maennchen@joshmartin.ch">>], <<"test">>},
																	 [{relay, <<"joshmartin.ch">>},
																	  {port, 25},
																	  {hostname, "smtp.covid19-tracing.ch"},
																	  {retries, 10}]
																	),
							 ?assertMatch({error, send, _}, Receipt),
							 ok
					 end
					}

All tests are passing, if this one is removes.

And, maybe this one should be added to a list with the previous testcase, because the former is currently ignored.

[seriyps]

So, did I get it correctly that before your change, if one passes UTF-8 encoded binary as, eg, content_type_params => [{<<"name">>, Name}], it will be encoded in some incorrect way?

[kape1395]

Yes, the MIME message was corrupted in the case of some letters in the filename. Two newlines were inserted instead of that letter (not sure was that by SMTP or Gmail client), so the attachment was corrupted and half of the headers were shown as a mail text body. With this change the filenames with UTF8 letters are processed correctly.

[mworrell]

@seriyps Shall we remove that test with smtp.covid19-tracing.ch ? I am not happy about using some external server to test our code. Especially as this also seems to be something that won't be there forever.

[mworrell]

@kape1395 I am terribly sorry, but could you rebase once again?

Ping me when you are done, then I wil merge.

[mworrell]

@kape1395 In #281 I have removed the test that relied on the external server. So after rebasing everything should pass.

[mworrell]

@kape1395 could you rebase this? Then we can merge.

[kape1395]

looking at it.

[kape1395]

It looks like it will be easier to reimplement my stuff than rebase it.
In the new master there is

rfc2047_q_encode(<<$\s, More/binary>>) ->
	% SPACE -> _
	<<$_, (rfc2047_q_encode(More))/binary>>;

It replaces space with "_". The proper approach is to replace it with "=20".
Is that accidental, or intended? If I change it to

rfc2047_q_encode(<<$\s, More/binary>>) ->
	% SPACE -> _
	<<"=20", (rfc2047_q_encode(More))/binary>>;

then other tests fail, but I think those tests are incorrect (because they expect an underscore instead of space).

[seriyps]

@kape1395 I think it was explained in the PR thatintroduced this change #294 (comment)

@Maria-12648430 do you have some more comments about it?

[kape1395]

I updated my changes to work with _ instead of =%20 and all the other changes in the master branch. I hope that looks good.

[juhlig]

@Maria-12648430 is currently not available, but I can answer that: see RFC 2047 Section 4.2 (2). Though it says some mail gateways may not understand the underscore, I never saw any problems with that (I work in the E-Mail business), which is not surprising given the RFC dates back to 1996.

[mworrell]

Thanks @kape1395 !