Fix problem with object identifiers in external modules

When parsing ASN.1, certain constructs can only be understood in
the full context of the entire ASN.1 module.

For instance, the value following ID in this simplified excerpt
from MTSAbstractService88:

administration-88 PORT ::= {
  ID {id-pt-administration 88}
}

the value following "ID" can be interpreted either as value
for:

  SEQUENCE {
    id-pt-administration INTEGER
  }

or as an OBJECT IDENTIFIER.

Our ASN.1 parser assumes that a SEQUENCE is meant, and if that
later turns out to be wrong, the SEQUENCE value is rewritten to
an OBJECT IDENTIFIER. The problem is that at the time of the
rewrite, we no longer know in which ASN.1 module id-pt-administration
was defined in, and we have to use the module name in the state{}
record. Unfortunately, the module name in the state{} record may
not always be correct.

While there are attempts in the code to keep the module name
up-to-date when checking imported types, it is not done consistently,
and it seems to be a difficult and error-prone task to attempt
to make it consistent.

A safer and less error-prone approach is to make sure that we
don't lose the module name while parsing. To make it clear what
we are doing, we will introduce a new #seqtag{} record that
are used for tags in SEQUENCE values. The name is based on its
primary use. The record also contains the module in case it happens
to be an OBJECT IDENTIFIER.

3 files changed, 36 insertions, 25 deletions

diff --git a/lib/asn1/src/asn1_records.hrl b/lib/asn1/src/asn1_records.hrl
index 396ba0fcfa..2540f0effc 100644
--- a/lib/asn1/src/asn1_records.hrl
+++ b/lib/asn1/src/asn1_records.hrl
@@ -73,6 +73,15 @@
 % Externalvaluereference -> modulename '.' typename
 -record('Externalvaluereference',{pos,module,value}).
 
+%% Used to hold a tag for a field in a SEQUENCE/SET. It can also
+%% be used for identifiers in OBJECT IDENTIFIER values, since the
+%% parser cannot always distinguish a SEQUENCE with one element from
+%% an OBJECT IDENTIFIER.
+-record(seqtag,
+	{pos :: integer(),
+	 module :: atom(),
+	 val :: atom()}).
+
 -record(state,{module,mname,type,tname,value,vname,erule,parameters=[],
 	       inputmodules,abscomppath=[],recordtopname=[],options,
 	       sourcedir}).
diff --git a/lib/asn1/src/asn1ct_check.erl b/lib/asn1/src/asn1ct_check.erl
index 33578153f0..fe50f1f3b0 100644
--- a/lib/asn1/src/asn1ct_check.erl
+++ b/lib/asn1/src/asn1ct_check.erl
@@ -2307,22 +2307,23 @@ validate_oid(_, S, OID, [Id|Vrest], Acc)
 		    error({value, {"illegal "++to_string(OID),[Id,Vrest],Acc}, S})
 	    end
     end;
-validate_oid(_, S, OID, [{Atom,Value}],[]) 
+validate_oid(_, S, OID, [{#seqtag{module=Mod,val=Atom},Value}], [])
   when is_atom(Atom),is_integer(Value) ->
     %% this case when an OBJECT IDENTIFIER value has been parsed as a 
     %% SEQUENCE value
-    Rec = #'Externalvaluereference'{module=S#state.mname,
+    Rec = #'Externalvaluereference'{module=Mod,
 				    value=Atom},
     validate_objectidentifier1(S, OID, [Rec,Value]);
-validate_oid(_, S, OID, [{Atom,EVRef}],[]) 
+validate_oid(_, S, OID, [{#seqtag{module=Mod,val=Atom},EVRef}], [])
   when is_atom(Atom),is_record(EVRef,'Externalvaluereference') ->
     %% this case when an OBJECT IDENTIFIER value has been parsed as a 
     %% SEQUENCE value OTP-4354
-    Rec = #'Externalvaluereference'{module=EVRef#'Externalvaluereference'.module,
+    Rec = #'Externalvaluereference'{module=Mod,
 				    value=Atom},
     validate_objectidentifier1(S, OID, [Rec,EVRef]);
-validate_oid(_, S, OID, [Atom|Rest],Acc) when is_atom(Atom) ->
-    Rec = #'Externalvaluereference'{module=S#state.mname,
+validate_oid(_, S, OID, [#seqtag{module=Mod,val=Atom}|Rest], Acc)
+  when is_atom(Atom) ->
+    Rec = #'Externalvaluereference'{module=Mod,
 				    value=Atom},
     validate_oid(true,S, OID, [Rec|Rest],Acc);
 validate_oid(_, S, OID, V, Acc) ->
@@ -2704,20 +2705,20 @@ normalize_set(S,Value,Components,NameList) ->
 	    normalized_record('SET',S,SortedVal,Components,NameList)
     end.
 
-sort_value(Components,Value) ->
-    ComponentNames = lists:map(fun(#'ComponentType'{name=Cname}) -> Cname end,
-			       Components),
-    sort_value1(ComponentNames,Value,[]).
-sort_value1(_,V=#'Externalvaluereference'{},_) ->
-    %% sort later, get the value in normalize_seq_or_set
-    V;
-sort_value1([N|Ns],Value,Acc) ->
-    case lists:keysearch(N,1,Value) of
-	{value,V} ->sort_value1(Ns,Value,[V|Acc]);
-	_ -> sort_value1(Ns,Value,Acc)
-    end;
-sort_value1([],_,Acc) ->
-    lists:reverse(Acc).
+sort_value(Components, Value0) when is_list(Value0) ->
+    {Keys0,_} = lists:mapfoldl(fun(#'ComponentType'{name=N}, I) ->
+				       {{N,I},I+1}
+			       end, 0, Components),
+    Keys = gb_trees:from_orddict(orddict:from_list(Keys0)),
+    Value1 = [{case gb_trees:lookup(N, Keys) of
+		   {value,K} -> K;
+		   none -> 'end'
+	       end,Pair} || {#seqtag{val=N},_}=Pair <- Value0],
+    Value = lists:sort(Value1),
+    [Pair || {_,Pair} <- Value];
+sort_value(_Components, #'Externalvaluereference'{}=Value) ->
+    %% Sort later.
+    Value.
 
 sort_val_if_set(['SET'|_],Val,Type) ->
     sort_value(Type,Val);
@@ -2750,9 +2751,9 @@ is_record_normalized(_S,Name,Value,NumComps) when is_tuple(Value) ->
 is_record_normalized(_,_,_,_) ->
     false.
 
-normalize_seq_or_set(SorS,S,[{Cname,V}|Vs],
+normalize_seq_or_set(SorS, S, [{#seqtag{val=Cname},V}|Vs],
 		     [#'ComponentType'{name=Cname,typespec=TS}|Cs],
-		     NameList,Acc) ->
+		     NameList, Acc) ->
     NewNameList = 
 	case TS#type.def of
 	    #'Externaltypereference'{type=TName} ->
diff --git a/lib/asn1/src/asn1ct_parser2.erl b/lib/asn1/src/asn1ct_parser2.erl
index 444c8969f1..f9c4387ca0 100644
--- a/lib/asn1/src/asn1ct_parser2.erl
+++ b/lib/asn1/src/asn1ct_parser2.erl
@@ -2866,13 +2866,14 @@ parse_SequenceValue(Tokens) ->
     throw({asn1_error,{get_line(hd(Tokens)),get(asn1_module),
 		       [got,get_token(hd(Tokens)),expected,'{']}}).
 
-parse_SequenceValue([{identifier,_,IdName}|Rest],Acc) ->
+parse_SequenceValue([{identifier,Pos,IdName}|Rest],Acc) ->
     {Value,Rest2} = parse_Value(Rest),
+    SeqTag = #seqtag{pos=Pos,module=get(asn1_module),val=IdName},
     case Rest2 of
 	[{',',_}|Rest3] ->
-	    parse_SequenceValue(Rest3,[{IdName,Value}|Acc]);
+	    parse_SequenceValue(Rest3, [{SeqTag,Value}|Acc]);
 	[{'}',_}|Rest3] ->
-	    {lists:reverse([{IdName,Value}|Acc]),Rest3};
+	    {lists:reverse(Acc, [{SeqTag,Value}]),Rest3};
 	_ ->
 	    throw({asn1_error,{get_line(hd(Rest2)),get(asn1_module),
 			       [got,get_token(hd(Rest2)),expected,'}']}})