Add service_opt() string_decode

To control whether stringish Diameter types are decoded to string or
left as binary. The motivation is the same as in the parent commit: to
avoid large strings being copied when incoming Diameter messages are
passed between processes; or *if* in the case of messages destined for
handle_request and handle_answer callbacks, since these are decoded in
the dedicated processes that the callbacks take place in. It would be
possible to do something about other messages without requiring an
option, but disabling the decode is the most effective.

The value is a boolean(), true being the default for backwards
compatibility. Setting false causes both diameter_caps records and
decoded messages to contain binary() in relevant places that previously
had string(): diameter_app(3) callbacks need to be prepared for the
change.

The Diameter types affected are OctetString and the derived types that
can contain arbitrarily large values: OctetString, UTF8String,
DiameterIdentity, DiameterURI, IPFilterRule, and QoSFilterRule. Time and
Address are unaffected.

The DiameterURI decode has been redone using re(3), which both
simplifies and does away with a vulnerability resulting from the
conversion of arbitrary strings to atom.

The solution continues the use and abuse of the process dictionary for
encode/decode purposes, last seen in commit 0f9cdba.

13 files changed, 303 insertions, 130 deletions

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index a5a99f7835..cb397614e5 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -899,6 +899,30 @@ Options <c>monitor</c> and <c>link</c> are ignored.</p>
 Defaults to the empty list.</p>
 </item>
 
+<marker id="string_decode"/>
+<tag><c>{string_decode, boolean()}</c></tag>
+<item>
+<p>
+Whether or not to decode AVPs of type &dict_OctetString; and its
+derived types &dict_DiameterIdentity;, &dict_DiameterURI;,
+&dict_IPFilterRule;, &dict_QoSFilterRule;, and &dict_UTF8String;.
+If <c>true</c> then AVPs of these types are decoded to string().
+If <c>false</c> then values are retained as binary().</p>
+
+<p>
+Defaults to <c>true</c>.</p>
+
+<warning>
+<p>
+This option should be set to <c>false</c>
+since a sufficiently malicious peer can otherwise cause large amounts
+of memory to be consumed when decoded Diameter messages are passed
+between processes.
+The default value is for backwards compatibility.</p>
+</warning>
+
+</item>
+
 <tag><c>{use_shared_peers, boolean() | [node()] | evaluable()}</c></tag>
 <item>
 <p>
diff --git a/lib/diameter/doc/src/diameter_dict.xml b/lib/diameter/doc/src/diameter_dict.xml
index 810a146b88..9db9bcffde 100644
--- a/lib/diameter/doc/src/diameter_dict.xml
+++ b/lib/diameter/doc/src/diameter_dict.xml
@@ -529,6 +529,11 @@ answer record and passed to a &app_handle_request;
 callback upon reception of an incoming request.</p>
 
 <p>
+In cases in which there is a choice between list() and binary() types
+for OctetString() and derived types, the representation is determined
+by the value of &mod_string_decode;.</p>
+
+<p>
 <em>Basic AVP Data Formats</em></p>
 
 <marker id="OctetString"/>
@@ -541,7 +546,7 @@ callback upon reception of an incoming request.</p>
 <marker id="Grouped"/>
 
 <pre>
-OctetString() = [0..255]
+OctetString() = string() | binary()
 Integer32()   = -2147483647..2147483647
 Integer64()   = -9223372036854775807..9223372036854775807
 Unsigned32()  = 0..4294967295
@@ -603,7 +608,7 @@ and <c>{{2104,2,26},{9,42,23}}</c> (both inclusive) can be encoded.</p>
 
 <marker id="UTF8String"/>
 <pre>
-UTF8String() = [integer()]
+UTF8String() = [integer()] | binary()
 </pre>
 
 <p>
diff --git a/lib/diameter/doc/src/seealso.ent b/lib/diameter/doc/src/seealso.ent
index 44541afb9b..b0e3a2c712 100644
--- a/lib/diameter/doc/src/seealso.ent
+++ b/lib/diameter/doc/src/seealso.ent
@@ -69,6 +69,8 @@ significant.
 <!ENTITY connect_timer '<seealso marker="#connect_timer">connect_timer</seealso>'>
 <!ENTITY watchdog_timer '<seealso marker="#watchdog_timer">watchdog_timer</seealso>'>
 
+<!ENTITY mod_string_decode '<seealso marker="diameter#service_opt">diameter:service_opt()</seealso> <seealso marker="diameter#string_decode">string_decode</seealso>'>
+
 <!-- diameter_app -->
 
 <!ENTITY app_handle_answer '<seealso marker="diameter_app#Mod:handle_answer-4">handle_answer/4</seealso>'>
@@ -102,6 +104,9 @@ significant.
 
 <!ENTITY dict_Address '<seealso marker="diameter_dict#DATA_TYPES">Address()</seealso>'>
 <!ENTITY dict_DiameterIdentity '<seealso marker="diameter_dict#DATA_TYPES">DiameterIdentity()</seealso>'>
+<!ENTITY dict_DiameterURI '<seealso marker="diameter_dict#DATA_TYPES">DiameterURI()</seealso>'>
+<!ENTITY dict_IPFilterRule '<seealso marker="diameter_dict#DATA_TYPES">IPFilterRule()</seealso>'>
+<!ENTITY dict_QoSFilterRule '<seealso marker="diameter_dict#DATA_TYPES">QoSFilterRule()</seealso>'>
 <!ENTITY dict_Grouped '<seealso marker="diameter_dict#DATA_TYPES">Grouped()</seealso>'>
 <!ENTITY dict_OctetString '<seealso marker="diameter_dict#DATA_TYPES">OctetString()</seealso>'>
 <!ENTITY dict_Time '<seealso marker="diameter_dict#DATA_TYPES">Time()</seealso>'>
diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 1bbdf6e34d..a3c259c651 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2014. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2015. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -306,6 +306,7 @@ call(SvcName, App, Message) ->
     | {restrict_connections, restriction()}
     | {sequence, sequence() | evaluable()}
     | {share_peers, remotes()}
+    | {string_decode, boolean()}
     | {use_shared_peers, remotes()}
     | {spawn_opt, list()}.
 
diff --git a/lib/diameter/src/base/diameter_capx.erl b/lib/diameter/src/base/diameter_capx.erl
index 93548ecafd..7dc61f229f 100644
--- a/lib/diameter/src/base/diameter_capx.erl
+++ b/lib/diameter/src/base/diameter_capx.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2015. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -50,7 +50,8 @@
 -export([build_CER/2,
          recv_CER/3,
          recv_CEA/3,
-         make_caps/2]).
+         make_caps/2,
+         binary_caps/1]).
 
 -include_lib("diameter/include/diameter.hrl").
 -include("diameter_internal.hrl").
@@ -115,7 +116,8 @@ mk_caps(Caps0, Opts) ->
 
 -define(SC(K,F),
         set_cap({K, Val}, {Caps, #diameter_caps{F = false} = C}) ->
-            {Caps#diameter_caps{F = cap(K, Val)}, C#diameter_caps{F = true}}).
+            {Caps#diameter_caps{F = cap(K, copy(Val))},
+             C#diameter_caps{F = true}}).
 
 ?SC('Origin-Host',         origin_host);
 ?SC('Origin-Realm',        origin_realm);
@@ -375,10 +377,10 @@ capx_to_caps(CEX, Dict) ->
                         'Firmware-Revision',
                         'AVP'],
                        CEX),
-    #diameter_caps{origin_host = OH,
-                   origin_realm = OR,
+    #diameter_caps{origin_host = copy(OH),
+                   origin_realm = copy(OR),
                    vendor_id = VId,
-                   product_name = PN,
+                   product_name = copy(PN),
                    origin_state_id = OSI,
                    host_ip_address = IP,
                    supported_vendor_id = SV,
@@ -389,6 +391,32 @@ capx_to_caps(CEX, Dict) ->
                    firmware_revision = FR,
                    avp = X}.
 
+%% Copy binaries to avoid retaining a reference to a large binary
+%% containing AVPs we aren't interested in.
+copy(B)
+  when is_binary(B) ->
+    binary:copy(B);
+
+copy(T) ->
+    T.
+
+%% binary_caps/1
+%%
+%% Encode stringish capabilities with {string_decode, false}.
+
+binary_caps(Caps) ->
+    lists:foldl(fun bcaps/2, Caps, [#diameter_caps.origin_host,
+                                    #diameter_caps.origin_realm,
+                                    #diameter_caps.product_name]).
+
+bcaps(N, Caps) ->
+    case element(N, Caps) of
+        undefined ->
+            Caps;
+        V ->
+            setelement(N, Caps, iolist_to_binary(V))
+    end.
+
 %% ---------------------------------------------------------------------------
 %% ---------------------------------------------------------------------------
 
diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index b4ecb63961..2de0dcf373 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2014. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2015. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -22,6 +22,8 @@
 -export([encode/2,
          decode/2,
          decode/3,
+         setopts/1,
+         getopt/1,
          collect_avps/1,
          decode_header/1,
          sequence_numbers/1,
@@ -59,6 +61,41 @@
 %%    +-+-+-+-+-+-+-+-+-+-+-+-+-
 
 %%% ---------------------------------------------------------------------------
+%%% # setopts/1
+%%% # getopt/1
+%%% ---------------------------------------------------------------------------
+
+%% These functions are a compromise in the same vein as the use of the
+%% process dictionary in diameter_gen.hrl in generated codec modules.
+%% Instead of rewriting the entire dictionary generation to pass
+%% encode/decode options around, the calling process sets them by
+%% calling setopts/1. At current, the only option is whether or not to
+%% decode binaries as strings, which is used by diameter_types.
+
+setopts(Opts)
+  when is_list(Opts) ->
+    lists:foreach(fun setopt/1, Opts).
+
+%% Decode stringish types to string()? The default true is for
+%% backwards compatibility.
+setopt({string_decode = K, false = B}) ->
+    setopt(K, B);
+
+setopt(_) ->
+    ok.
+
+setopt(Key, Value) ->
+    put({diameter, Key}, Value).
+
+getopt(Key) ->
+    case get({diameter, Key}) of
+        undefined when Key == string_decode ->
+            true;
+        V ->
+            V
+    end.
+
+%%% ---------------------------------------------------------------------------
 %%% # encode/2
 %%% ---------------------------------------------------------------------------
 
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index c0a4f7df69..e446f7c479 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -642,13 +642,23 @@ make_config(SvcName, Opts) ->
                          {false, monitor},
                          {?NOMASK, sequence},
                          {nodes, restrict_connections},
+                         {true, string_decode},
                          {[], spawn_opt}]),
 
+    D = proplists:get_value(string_decode, SvcOpts, true),
+
     #service{name = SvcName,
              rec = #diameter_service{applications = Apps,
-                                     capabilities = Caps},
+                                     capabilities = binary_caps(Caps, D)},
              options = SvcOpts}.
 
+binary_caps(Caps, true) ->
+    Caps;
+binary_caps(Caps, false) ->
+    diameter_capx:binary_caps(Caps).
+
+%% make_opts/2
+
 make_opts(Opts, Defs) ->
     Known = [{K, get_opt(K, Opts, D)} || {D,K} <- Defs],
     Unknown = Opts -- Known,
@@ -667,7 +677,8 @@ opt(K, false = B)
 
 opt(K, true = B)
   when K == share_peers;
-       K == use_shared_peers ->
+       K == use_shared_peers;
+       K == string_decode ->
     B;
 
 opt(restrict_connections, T)
diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index ee6e7dd89e..cbf601f6f5 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2014. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2015. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -138,7 +138,8 @@
 %% # start/3
 %% ---------------------------------------------------------------------------
 
--spec start(T, [Opt], {diameter:sequence(),
+-spec start(T, [Opt], {[diameter:service_opt()]
+                       | diameter:sequence(),  %% from old code
                        [node()],
                        module(),
                        #diameter_service{}})
@@ -177,10 +178,15 @@ init(T) ->
     proc_lib:init_ack({ok, self()}),
     gen_server:enter_loop(?MODULE, [], i(T)).
 
-i({Ack, WPid, {M, Ref} = T, Opts, {Mask, Nodes, Dict0, Svc}}) ->
+i({Ack, WPid, T, Opts, {{_,_} = Mask, Nodes, Dict0, Svc}}) -> %% from old code
+    i({Ack, WPid, T, Opts, {[{sequence, Mask}], Nodes, Dict0, Svc}});
+
+i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) ->
     erlang:monitor(process, WPid),
     wait(Ack, WPid),
     diameter_stats:reg(Ref),
+    diameter_codec:setopts(SvcOpts),
+    {_,_} = Mask = proplists:get_value(sequence, SvcOpts),
     {[Cs,Ds], Rest} = proplists:split(Opts, [capabilities_cb, disconnect_cb]),
     putr(?CB_KEY, {Ref, [F || {_,F} <- Cs]}),
     putr(?DPR_KEY, [F || {_, F} <- Ds]),
@@ -699,6 +705,8 @@ build_answer('CER',
              = Pkt,
              #state{dictionary = Dict0}
              = S) ->
+    diameter_codec:setopts([{string_decode, false}]),
+
     {SupportedApps, RCaps, CEA} = recv_CER(CER, S),
 
     [RC, IS] = Dict0:'#get-'(['Result-Code', 'Inband-Security-Id'], CEA),
@@ -886,6 +894,8 @@ handle_CEA(#diameter_packet{header = H}
         = DPkt
         = diameter_codec:decode(Dict0, Pkt),
 
+    diameter_codec:setopts([{string_decode, false}]),
+
     RC = result_code(incr_rc(recv, DPkt, Dict0)),
 
     {SApps, IS, RCaps} = recv_CEA(DPkt, S),
diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index 04401a3d87..a01bcdd4e7 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -130,7 +130,8 @@
          :: [{sequence, diameter:sequence()}  %% sequence mask
              | {share_peers, diameter:remotes()}       %% broadcast to
              | {use_shared_peers, diameter:remotes()}  %% use from
-             | {restrict_connections, diameter:restriction()}]}).
+             | {restrict_connections, diameter:restriction()}
+             | {string_decode, boolean()}]}).
 %% shared_peers reflects the peers broadcast from remote nodes.
 
 %% Record representing an RFC 3539 watchdog process implemented by
@@ -261,16 +262,22 @@ whois(SvcName) ->
 %% ---------------------------------------------------------------------------
 
 -spec pick_peer(SvcName, AppOrAlias, Opts)
-   -> {{TPid, Caps, App}, Mask}
-    | false
-    | {error, term()}
+   -> {{TPid, Caps, App}, Mask, SvcOpts}
+    | false  %% no selection
+    | {error, no_service}
  when SvcName :: diameter:service_name(),
-      AppOrAlias :: {alias, diameter:app_alias()} | #diameter_app{},
-      Opts :: tuple(),
+      AppOrAlias :: #diameter_app{}
+                  | {alias, diameter:app_alias()},
+      Opts :: {fun((Dict :: module()) -> [term()]),
+               diameter:peer_filter(),
+               Xtra :: list()},
       TPid :: pid(),
       Caps :: #diameter_caps{},
       App  :: #diameter_app{},
-      Mask :: diameter:sequence().
+      Mask :: diameter:sequence(),
+      SvcOpts :: [diameter:service_opt()].
+%% Extract Mask in the returned tuple so that diameter_traffic doesn't
+%% need to know about the ordering of SvcOpts used here.
 
 pick_peer(SvcName, App, Opts) ->
     pick(lookup_state(SvcName), App, Opts).
@@ -287,10 +294,10 @@ pick(#state{service = #diameter_service{applications = Apps}}
      Opts) ->  %% initial call from diameter:call/4
     pick(S, find_outgoing_app(Alias, Apps), Opts);
 
-pick(_, false, _) ->
-    false;
+pick(_, false = No, _) ->
+    No;
 
-pick(#state{options = [{_, Mask} | _]}
+pick(#state{options = [{_, Mask} | SvcOpts]}
      = S,
      #diameter_app{module = ModX, dictionary = Dict}
      = App0,
@@ -299,7 +306,7 @@ pick(#state{options = [{_, Mask} | _]}
     [_,_] = RealmAndHost = diameter_lib:eval([DestF, Dict]),
     case pick_peer(App, RealmAndHost, Filter, S) of
         {TPid, Caps} ->
-            {{TPid, Caps, App}, Mask};
+            {{TPid, Caps, App}, Mask, SvcOpts};
         false = No ->
             No
     end.
@@ -690,7 +697,8 @@ service_options(Opts) ->
      {restrict_connections, proplists:get_value(restrict_connections,
                                                 Opts,
                                                 ?RESTRICT)},
-     {spawn_opt, proplists:get_value(spawn_opt, Opts, [])}].
+     {spawn_opt, proplists:get_value(spawn_opt, Opts, [])},
+     {string_decode, proplists:get_value(string_decode, Opts, true)}].
 %% The order of options is significant since we match against the list.
 
 mref(false = No) ->
@@ -802,10 +810,13 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT,
   when Type == connect;
        Type == accept ->
     #diameter_service{applications = Apps}
-        = Svc
+        = Svc1
         = merge_service(Opts, Svc0),
-    {_,_} = Mask = proplists:get_value(sequence, SvcOpts),
-    RecvData = diameter_traffic:make_recvdata([SvcName, PeerT, Apps, Mask]),
+    Svc = binary_caps(Svc1, proplists:get_value(string_decode, SvcOpts, true)),
+    RecvData = diameter_traffic:make_recvdata([SvcName,
+                                               PeerT,
+                                               Apps,
+                                               SvcOpts]),
     T = {{spawn_opts([Opts, SvcOpts]), RecvData}, Opts, SvcOpts, Svc},
     Rec = #watchdog{type = Type,
                     ref = Ref,
@@ -816,8 +827,13 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT,
                         [],
                         N).
 
+binary_caps(Svc, true) ->
+    Svc;
+binary_caps(#diameter_service{capabilities = Caps} = Svc, false) ->
+    Svc#diameter_service{capabilities = diameter_capx:binary_caps(Caps)}.
+
 wd(Type, Ref, T, WatchdogT, Rec) ->
-    Pid = wd(Type, Ref, T),
+    Pid = start_watchdog(Type, Ref, T),
     insert(WatchdogT, Rec#watchdog{pid = Pid}),
     Pid.
 
@@ -831,7 +847,7 @@ spawn_opts(Optss) ->
           T /= link,
           T /= monitor].
 
-wd(Type, Ref, T) ->
+start_watchdog(Type, Ref, T) ->
     {_MRef, Pid} = diameter_watchdog:start({Type, Ref}, T),
     Pid.
 
@@ -852,7 +868,7 @@ ms({applications, As}, #diameter_service{applications = Apps} = S)
 
 %% The fact that all capabilities can be configured on the transports
 %% means that the service doesn't necessarily represent a single
-%% locally implemented Diameter peer as identified by Origin-Host: a
+%% locally implemented Diameter node as identified by Origin-Host: a
 %% transport can configure its own Origin-Host. This means that the
 %% service little more than a placeholder for default capabilities
 %% plus a list of applications that individual transports can choose
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 18c1965f77..a9dc46ea31 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -77,7 +77,8 @@
         {peerT        :: ets:tid(),
          service_name :: diameter:service_name(),
          apps         :: [#diameter_app{}],
-         sequence     :: diameter:sequence()}).
+         sequence     :: diameter:sequence(),
+         codec        :: list()}).
 
 %% Record stored in diameter_request for each outgoing request.
 -record(request,
@@ -92,11 +93,16 @@
 %% # make_recvdata/1
 %% ---------------------------------------------------------------------------
 
-make_recvdata([SvcName, PeerT, Apps, Mask | _]) ->
+make_recvdata([SvcName, PeerT, Apps, {_,_} = Mask | _]) ->  %% from old code
+    make_recvdata([SvcName, PeerT, Apps, [{sequence, Mask}]]);
+
+make_recvdata([SvcName, PeerT, Apps, SvcOpts | _]) ->
+    {_,_} = Mask = proplists:get_value(sequence, SvcOpts),
     #recvdata{service_name = SvcName,
               peerT = PeerT,
               apps = Apps,
-              sequence = Mask}.
+              sequence = Mask,
+              codec = [T || {K,_} = T <- SvcOpts, K == string_decode]}.
 
 %% ---------------------------------------------------------------------------
 %% peer_up/1
@@ -270,8 +276,11 @@ recv_request(TPid,
              #diameter_packet{header = #diameter_header{application_id = Id}}
              = Pkt,
              Dict0,
-             #recvdata{peerT = PeerT, apps = Apps}
+             #recvdata{peerT = PeerT,
+                       apps = Apps,
+                       codec = Opts}
              = RecvData) ->
+    diameter_codec:setopts(Opts),
     send_A(recv_R(diameter_service:find_incoming_app(PeerT, TPid, Id, Apps),
                   TPid,
                   Pkt,
@@ -279,7 +288,13 @@ recv_request(TPid,
                   RecvData),
            TPid,
            Dict0,
-           RecvData).
+           RecvData);
+
+recv_request(TPid, Pkt, Dict0, RecvData) -> %% from old code
+    recv_request(TPid,
+                 Pkt,
+                 Dict0,
+                 #recvdata{} = erlang:append_element(RecvData, [])).
 
 %% recv_R/5
 
@@ -1225,10 +1240,9 @@ answer_rc(_, _, Sent) ->
 
 send_R(SvcName, AppOrAlias, Msg, Opts, Caller) ->
     case pick_peer(SvcName, AppOrAlias, Msg, Opts) of
-        {{_,_,_} = Transport, Mask} ->
+        {Transport, Mask, SvcOpts} ->
+            diameter_codec:setopts(SvcOpts),
             send_request(Transport, Mask, Msg, Opts, Caller, SvcName);
-        false ->
-            {error, no_connection};
         {error, _} = No ->
             No
     end.
@@ -1290,6 +1304,8 @@ send_request({TPid, Caps, App}
            SvcName,
            []).
 
+%% send_R/7
+
 send_R({send, Msg}, Pkt, Transport, Opts, Caller, SvcName, Fs) ->
     send_R(make_request_packet(Msg, Pkt),
            Transport,
@@ -1550,7 +1566,9 @@ a(Hdr, SvcName, discard) ->
 %% timer value is ignored. This means that an answer could be accepted
 %% from a peer after timeout in the case of failover.
 
-retransmit({{_,_,App} = Transport, _Mask}, Req, Opts, SvcName, Timeout) ->
+%% retransmit/5
+
+retransmit({{_,_,App} = Transport, _, _}, Req, Opts, SvcName, Timeout) ->
     try retransmit(Transport, Req, SvcName, Timeout) of
         T -> recv_A(Timeout, SvcName, App, Opts, T)
     catch
@@ -1571,17 +1589,26 @@ pick_peer(SvcName,
     pick_peer(SvcName, App, Msg, Opts#options{extra = []});
 
 pick_peer(_, _, undefined, _) ->
-    false;
+    {error, no_connection};
 
 pick_peer(SvcName,
           AppOrAlias,
           Msg,
           #options{filter = Filter, extra = Xtra}) ->
-    diameter_service:pick_peer(SvcName,
-                               AppOrAlias,
-                               {fun(D) -> get_destination(D, Msg) end,
-                                Filter,
-                                Xtra}).
+    pick(diameter_service:pick_peer(SvcName,
+                                    AppOrAlias,
+                                    {fun(D) -> get_destination(D, Msg) end,
+                                     Filter,
+                                     Xtra})).
+
+pick({{_,_,_} = Transport, Mask}) ->  %% from old code; dialyzer complains
+    {Transport, Mask, []};            %% about this
+
+pick(false) ->
+    {error, no_connection};
+
+pick(T) ->
+    T.
 
 %% handle_error/4
 
diff --git a/lib/diameter/src/base/diameter_types.erl b/lib/diameter/src/base/diameter_types.erl
index 442d90c98b..28a0635c57 100644
--- a/lib/diameter/src/base/diameter_types.erl
+++ b/lib/diameter/src/base/diameter_types.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2015. All Rights Reserved.
 %%
 %% The contents of this file are subject to the Erlang Public License,
 %% Version 1.1, (the "License"); you may not use this file except in
@@ -90,7 +90,12 @@
 
 'OctetString'(decode, Bin)
   when is_binary(Bin) ->
-    binary_to_list(Bin);
+    case diameter_codec:getopt(string_decode) of
+        true ->
+            binary_to_list(Bin);
+        _ ->
+            Bin
+    end;
 
 'OctetString'(decode, B) ->
     ?INVALID_LENGTH(B);
@@ -298,17 +303,19 @@
     'OctetString'(M, lists:duplicate(0,7));
 
 'DiameterURI'(encode, #diameter_uri{type = Type,
-                                    fqdn = D,
-                                    port = P,
+                                    fqdn = DN,
+                                    port = PN,
                                     transport = T,
-                                    protocol = Prot}
-                      = U) ->
-    S = lists:append([atom_to_list(Type), "://", D,
-                      ":", integer_to_list(P),
+                                    protocol = P})
+  when (Type == 'aaa' orelse Type == 'aaas'),
+       is_integer(PN),
+       0 =< PN,
+       (T == tcp orelse T == sctp orelse T == udp),
+       (P == diameter orelse P == radius orelse P == 'tacacs+') ->
+    iolist_to_binary([atom_to_list(Type), "://", DN,
+                      ":", integer_to_list(PN),
                       ";transport=", atom_to_list(T),
-                      ";protocol=", atom_to_list(Prot)]),
-    U = scan_uri(S), %% assert
-    list_to_binary(S);
+                      ";protocol=", atom_to_list(P)]);
 
 'DiameterURI'(encode, Str) ->
     Bin = iolist_to_binary(Str),
@@ -321,7 +328,6 @@
 'IPFilterRule'(encode = M, zero) ->
     'OctetString'(M, lists:duplicate(0,33));
 
-%% TODO: parse grammar.
 'IPFilterRule'(M, X) ->
     'OctetString'(M, X).
 
@@ -331,7 +337,6 @@
 'QoSFilterRule'(encode = M, zero = X) ->
     'IPFilterRule'(M, X);
 
-%% TODO: parse grammar.
 'QoSFilterRule'(M, X) ->
     'OctetString'(M, X).
 
@@ -339,7 +344,13 @@
 
 'UTF8String'(decode, Bin)
   when is_binary(Bin) ->
-    tl([0|_] = unicode:characters_to_list([0, Bin])); %% assert list return
+    case diameter_codec:getopt(string_decode) of
+        true ->
+            %% assert list return
+            tl([0|_] = unicode:characters_to_list([0, Bin]));
+        false ->
+            <<_/binary>> = unicode:characters_to_binary(Bin)
+    end;
 
 'UTF8String'(decode, B) ->
     ?INVALID_LENGTH(B);
@@ -507,55 +518,42 @@ msb(false) -> ?TIME_2036.
 %%
 %%       aaa-protocol       = ( "diameter" / "radius" / "tacacs+" )
 
-scan_uri(Bin)
-  when is_binary(Bin) ->
-    scan_uri(binary_to_list(Bin));
-scan_uri("aaa://" ++ Rest) ->
-    scan_fqdn(Rest, #diameter_uri{type = aaa});
-scan_uri("aaas://" ++ Rest) ->
-    scan_fqdn(Rest, #diameter_uri{type = aaas}).
-
-scan_fqdn(S, U) ->
-    {[_|_] = F, Rest} = lists:splitwith(fun is_fqdn/1, S),
-    scan_opt_port(Rest, U#diameter_uri{fqdn = F}).
-
-scan_opt_port(":" ++ S, U) ->
-    {[_|_] = P, Rest} = lists:splitwith(fun is_digit/1, S),
-    scan_opt_transport(Rest, U#diameter_uri{port = list_to_integer(P)});
-scan_opt_port(S, U) ->
-    scan_opt_transport(S, U).
-
-scan_opt_transport(";transport=" ++ S, U) ->
-    {P, Rest} = transport(S),
-    scan_opt_protocol(Rest, U#diameter_uri{transport = P});
-scan_opt_transport(S, U) ->
-    scan_opt_protocol(S, U).
-
-scan_opt_protocol(";protocol=" ++ S, U) ->
-    {P, ""} = protocol(S),
-    U#diameter_uri{protocol = P};
-scan_opt_protocol("", U) ->
-    U.
-
-transport("tcp" ++ S) ->
-    {tcp, S};
-transport("sctp" ++ S) ->
-    {sctp, S};
-transport("udp" ++ S) ->
-    {udp, S}.
-
-protocol("diameter" ++ S) ->
-    {diameter, S};
-protocol("radius" ++ S) ->
-    {radius, S};
-protocol("tacacs+" ++ S) ->
-    {'tacacs+', S}.
-
-is_fqdn(C) ->
-    is_digit(C) orelse is_alpha(C) orelse C == $. orelse C == $-.
-
-is_alpha(C) ->
-    ($a =< C andalso C =< $z) orelse ($A =< C andalso C =< $Z).
-
-is_digit(C) ->
-    $0 =< C andalso C =< $9.
+scan_uri(Bin) ->
+    RE = "^(aaas?)://"
+         "([-a-zA-Z0-9.]+)"
+         "(:([0-9]+))?"
+         "(;transport=(tcp|sctp|udp))?"
+         "(;protocol=(diameter|radius|tacacs\\+))?$",
+    {match, [A, DN, PN, T, P]} = re:run(Bin,
+                                        RE,
+                                        [{capture, [1,2,4,6,8], binary}]),
+    #diameter_uri{port = PN0,
+                  transport = T0,
+                  protocol = P0}
+        = #diameter_uri{},
+    #diameter_uri{type = to_atom(A),
+                  fqdn = from_bin(DN),
+                  port = to_int(PN, PN0),
+                  transport = to_atom(T, T0),
+                  protocol = to_atom(P, P0)}.
+
+from_bin(B) ->
+    case diameter_codec:getopt(string_decode) of
+        true ->
+            binary_to_list(B);
+        false ->
+            B
+    end.
+
+to_int(<<>>, N) ->
+    N;
+to_int(B, _) ->
+    binary_to_integer(B).
+
+to_atom(<<>>, A) ->
+    A;
+to_atom(B, _) ->
+    to_atom(B).
+
+to_atom(B) ->
+    binary_to_atom(B, latin1).
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index 8223b7df98..d14ddb758b 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -124,14 +124,15 @@ i({Ack, T, Pid, {RecvData,
     wait(Ack, Pid),
     {_, Seed} = diameter_lib:seed(),
     random:seed(Seed),
-    putr(restart, {T, Opts, Svc}),  %% save seeing it in trace
-    putr(dwr, dwr(Caps)),           %%
+    putr(restart, {T, Opts, Svc, SvcOpts}),  %% save seeing it in trace
+    putr(dwr, dwr(Caps)),                    %%
+    diameter_codec:setopts([{string_decode, false}]),
     {_,_} = Mask = proplists:get_value(sequence, SvcOpts),
     Restrict = proplists:get_value(restrict_connections, SvcOpts),
     Nodes = restrict_nodes(Restrict),
     Dict0 = common_dictionary(Apps),
     #watchdog{parent = Pid,
-              transport = start(T, Opts, Mask, Nodes, Dict0, Svc),
+              transport = start(T, Opts, SvcOpts, Nodes, Dict0, Svc),
               tw = proplists:get_value(watchdog_timer,
                                        Opts,
                                        ?DEFAULT_TW_INIT),
@@ -166,11 +167,11 @@ config({okay, N}, Rec)
   when ?IS_NATURAL(N) ->
     Rec#config{okay = N}.
 
-%% start/5
+%% start/6
 
-start(T, Opts, Mask, Nodes, Dict0, Svc) ->
+start(T, Opts, SvcOpts, Nodes, Dict0, Svc) ->
     {_MRef, Pid}
-        = diameter_peer_fsm:start(T, Opts, {Mask, Nodes, Dict0, Svc}),
+        = diameter_peer_fsm:start(T, Opts, {SvcOpts, Nodes, Dict0, Svc}),
     Pid.
 
 %% common_dictionary/1
@@ -320,7 +321,7 @@ code_change(_, State, _) ->
 %% expiry; or another watchdog is saying the same after reestablishing
 %% a connection previously had by this one.
 transition(close, #watchdog{}) ->
-    {{accept, _}, _, _} = getr(restart), %% assert
+    {accept, _} = role(), %% assert
     stop;
 
 %% Service is asking for the peer to be taken down gracefully.
@@ -369,7 +370,7 @@ transition({open, TPid, Hosts, _} = Open,
                      restrict = {_,R},
                      config = #config{suspect = OS}}
            = S) ->
-    case okay(getr(restart), Hosts, R) of
+    case okay(role(), Hosts, R) of
         okay ->
             set_watchdog(S#watchdog{status = okay,
                                     num_dwa = OS});
@@ -423,7 +424,7 @@ transition({'DOWN', _, process, TPid, _Reason} = D,
            = S0) ->
     S = S0#watchdog{pending = false,
                     transport = undefined},
-    {{M,_}, _, _} = getr(restart),
+    {M,_} = role(),
 
     %% Close an accepting watchdog immediately if there's no
     %% restriction on the number of connections to the same peer: the
@@ -490,7 +491,7 @@ encode(dwa, Dict0, #diameter_packet{header = H, transport_data = TD}
 
 %% okay/3
 
-okay({{accept, Ref}, _, _}, Hosts, Restrict) ->
+okay({accept, Ref}, Hosts, Restrict) ->
     T = {?MODULE, connection, Ref, Hosts},
     diameter_reg:add(T),
     if Restrict ->
@@ -501,7 +502,7 @@ okay({{accept, Ref}, _, _}, Hosts, Restrict) ->
 %% Register before matching so that at least one of two registering
 %% processes will match the other.
 
-okay({{connect, _}, _, _}, _, _) ->
+okay({connect, _}, _, _) ->
     okay.
 
 %% okay/2
@@ -516,6 +517,11 @@ okay(C) ->
     [_|_] = [send(P, close) || {_,P} <- C, self() /= P],
     reopen.
 
+%% role/0
+
+role() ->
+    element(1, getr(restart)).
+
 %% set_watchdog/1
 
 set_watchdog(#watchdog{tw = TwInit,
@@ -801,26 +807,28 @@ restart(S) ->  %% reconnect has won race with timeout
 %% state down rather then initial when receiving notification of an
 %% open connection.
 
-restart({{connect, _} = T, Opts, Svc},
+restart({T, Opts, Svc}, S) ->  %% put in old code
+    restart({T, Opts, Svc, []}, S);
+
+restart({{connect, _} = T, Opts, Svc, SvcOpts},
         #watchdog{parent = Pid,
-                  sequence = Mask,
                   restrict = {R,_},
                   dictionary = Dict0}
         = S) ->
     send(Pid, {reconnect, self()}),
     Nodes = restrict_nodes(R),
-    S#watchdog{transport = start(T, Opts, Mask, Nodes, Dict0, Svc),
+    S#watchdog{transport = start(T, Opts, SvcOpts, Nodes, Dict0, Svc),
                restrict = {R, lists:member(node(), Nodes)}};
 
 %% No restriction on the number of connections to the same peer: just
 %% die. Note that a state machine never enters state REOPEN in this
 %% case.
-restart({{accept, _}, _, _}, #watchdog{restrict = {_, false}}) ->
+restart({{accept, _}, _, _, _}, #watchdog{restrict = {_, false}}) ->
     stop;  %% 'DOWN' was in old code: 'close' was not sent
 
 %% Otherwise hang around until told to die, either by the service or
 %% by another watchdog.
-restart({{accept, _}, _, _}, S) ->
+restart({{accept, _}, _, _, _}, S) ->
     S.
 
 %% Don't currently use Opts/Svc in the accept case.
diff --git a/lib/diameter/test/diameter_config_SUITE.erl b/lib/diameter/test/diameter_config_SUITE.erl
index ad5b3f9420..642fe2adb4 100644
--- a/lib/diameter/test/diameter_config_SUITE.erl
+++ b/lib/diameter/test/diameter_config_SUITE.erl
@@ -82,6 +82,9 @@
            [false],
            [[node(), node()]]],
           [[x]]},
+         {string_decode,
+          [[true], [false]],
+          [[0], [x]]},
          {invalid_option,  %% invalid service options are rejected
           [],
           [[x],