Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Update smtp_util.erl #44

Open
wants to merge 1 commit into from

5 participants

@Gertm

Tried this with Erlang R16B02 and it failed to compile, this fixes it.

@mworrell
Collaborator

This is indeed a change in the R16 (and later) crypto library.
Problem is that the hmac/3 and hmac/4 functions are not available in R15 and earlier.

We might want to either add conditional compilation or a function that tests on the availability of either crypto function.

See also basho/velvet#9 where the same crypto hmac issue was addressed.

@Gertm

We could do it this way for line 65:

case proplists:is_defined(hmac, proplists:get_value(exports, crypto:module_info())) of
true -> Bin = crypto:hmac(md5, Key, Data);
false -> Bin = crypto:md5_mac(Key, Data)
end,

But I feel like there should be a better way.

@mworrell
Collaborator

Maybe something like the following code in velvet?

%% This is for compatibility for R16B and R15B
%% R15B does not have crypto:hmac/3, which looks
%% newly added in R16B. in future crypto:hmac_final/1
%% will probably replaced with crypto:hmac/3
%% like crypto:hmac(sha, KeyData, STS)
Ctx = crypto:hmac_update(crypto:hmac_init(sha, KeyData), STS),
base64:encode_to_string(crypto:hmac_final(Ctx)).

Of course with the base64 encode, and with md5 instead of sha.

@imtal

To my opinion these differences in environment should actually be handled at compile time. So the Erlang version should be tested in ./configure or the Makefile and a macro should be passed to the compiler. The precompiler then can use -ifdef or -ifndef to put the right version in place. Agree?

@mworrell
Collaborator

I am for a generic solution, with as little ifdefs as possible.
That keeps the code cleaner and easier to maintain.

That said it is possible to use rebar to check for the Erlang version and set compile flags accordingly.

@imtal

The (minimal) amount of ifdefs is not an indicator to me. It is more that to my opinion version differences should be handled at compile time and not at run time. That said, I would be really glad to help with this change but do no yet have the appropriate knowledge of the sources. Sorry. It is just that I have a 'watch' on this project because I am working on a imapd implementation using the rfc822 module from here ...

@mworrell
Collaborator

I agree that the cleanest solution is some defines in rebar, together with conditional compilation.

@AeroNotix

The real way to do it is stop trying to support everything under the sun and just tag a breaking release and allow clients to use whatever version they are dependent on.

This backwards compatibility fever is just yak shaving in disguise.

@Vagabond
Owner

I would say on this, that supporting the last 2 MAJOR releases should be the goal. With R17 approaching, we should be able to drop R15 support soon.

@mworrell
Collaborator

:+1:

@arjan @kaos Any comments?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 21, 2013
  1. @Gertm

    Update smtp_util.erl

    Gertm authored
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  src/smtp_util.erl
View
2  src/smtp_util.erl
@@ -62,7 +62,7 @@ guess_FQDN() ->
%% @doc Compute the CRAM digest of `Key' and `Data'
-spec(compute_cram_digest/2 :: (Key :: binary(), Data :: string()) -> binary()).
compute_cram_digest(Key, Data) ->
- Bin = crypto:md5_mac(Key, Data),
+ Bin = crypto:hmac(md5, Key, Data),
list_to_binary([io_lib:format("~2.16.0b", [X]) || <<X>> <= Bin]).
%% @doc Generate a seed string for CRAM.
Something went wrong with that request. Please try again.