[Home]

Summary:ASTERISK-07318: [patch] ENUMLOOKUP function can't handle multiple records with the same order and priority
Reporter:Luke-Jr (luke-jr)Labels:
Date Opened:2006-07-10 11:22:41Date Closed:2006-07-15 18:38:05
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Functions/func_enum
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20060713__bug7513.diff.txt
( 1) 20060715__bug7513__minus_whitespace.diff.txt
Description:If multiple ENUM records have the same order and priority, Asterisk replaces the subsequent entries with the first one.
For example, if I have an ENUM with 4 entries:
Order: 1 Pri: 1 IAX
Order: 1 Pri: 1 SIP
Order: 1 Pri: 2 mailto
and use ENUMLOOKUP to iterate over the 3 (the count returns correctly), I get:
IAX
IAX
mailto
Comments:By: Serge Vecher (serge-v) 2006-07-10 12:19:07

1. What about 1.2.9.1?
2. Can you please provide an excerpt from dialplan and respective console log?
By: Luke-Jr (luke-jr) 2006-07-10 13:16:01

macro route-enum(exten, IsPOTS, Timeout, DialOpts) {
// based on http://www.voip-info.org/wiki/view/RFC+Compliant+ENUM+Macro
Set(DIAL_NUMBER=${exten});

Set(E164NETWORKS=e164.arpa-e164.info-e164.org);
if ("${ENUMDEPTH}" = "")
ENUMDEPTH=0;
ENUMDEPTH=1 + ${ENUMDEPTH};
if (${ENUMDEPTH} > 5) { // max enum depth we will go
// This is to prevent a looping enum entry; there is no other safe way to be sure we don't get stuck in such a trap.
Set(DIALSTATUS=EnumMaxDepth);
goto end;
};
if ("${DIAL_NUMBER:0:1}" != "+")
Set(DIAL_NUMBER=+${DIAL_NUMBER}); // Add a plus to the start, becasue ENUMLOOKUP needs it.
// Checking here to see if there are any e164 networks left to check.
for (&iterator(${E164NETWORKS},ENUMNET); "${ENUMNET}" != ""; &iterate(ENUMNET)) {
// OK, this is now quite complex. To remain compliant, we have to iterate through, in order, the returned records. Since we want to make this call over the network, we can ignore tel: lines. Even if it's first priority.
Set(ENUMCOUNT=${ENUMLOOKUP(${DIAL_NUMBER},ALL,c,${ENUMNET})});
// Documentation is wrong. It can return nothing if the enum lookup fails. Grr. Now the count may be zero, so if it is, check the next network
if ("${ENUMCOUNT}" = "")
ENUMCOUNT=0;
// Now, let's start through them.
for (ENUMPTR=1; ${ENUMPTR} <= ${ENUMCOUNT}; ENUMPTR=${ENUMPTR} + 1) {
Set(ENUM=${ENUMLOOKUP(${DIAL_NUMBER},ALL,${ENUMPTR},${ENUMNET})});
// Sanity check the return, make sure there's something in there.
if (${LEN(${ENUM})} = 0)
continue;
NoOP(Enum result: ${ENUM});
Set(DIALSTR=);
Set(DIALSTATUS=);
switch(${CUT(ENUM,:,1)}) {
case sip:
// If the prefix is 'sip:'...
Set(DIALSTR=SIP/${ENUM:4});
break;
case iax2:
// If it's IAX2...
Set(DIALSTR=IAX2/${ENUM:5});
break;
case h323:
// It doesn't matter if you don't have h323 enabled, as when it tries to dial, it cares about dialstatus and retries if there are any enum results left.
// Or even if it's H323.
Set(DIALSTR=H323/${ENUM:5});
break;
case tel:
// redirect to another number... potentially dangerous (looping)
&enumtest(${ENUM:4});
break;
default:
// If we're here, it's not a protocol we know about. Let's increment the pointer and if it's more than ENUMCOUNT, we know we've run out of options. Try the next e164 network.
Set(DIALSTATUS=CHANUNAVAIL);
break;
};
if ("${DIALSTR}" != "") {
Dial(${DIALSTR},${Timeout},o${DialOpts});
NoOp(Dial exited in enumtest with ${DIALSTATUS});
};
// Now, if we're still here, that means the Dial failed for some reason.
// If it's CONGESTION or CHANUNAVAIL we probably want to try again on a different channel. However, if it's the last one, we don't have any left, and I didn't keep any previous dialstatuses, so hopefully someone looking throught the logs would have seen the NoOp's
if ("${DIALSTATUS}" != "CONGESTION" & "${DIALSTATUS}" != "CHANUNAVAIL") {
NoOp(Enum Dial(${DIALSTR}) failed due to ${DIALSTATUS});
goto end;
};
};
};
end:
NoOp(EnumLookups failed);
};


399 => {
NoOP(${ENUMLOOKUP(+1xxxxxxxxxx,ALL,1,e164.org)});
NoOP(${ENUMLOOKUP(+1xxxxxxxxxx,ALL,2,e164.org)});
NoOP(${ENUMLOOKUP(+1xxxxxxxxxx,ALL,3,e164.org)});
NoOP(${ENUMLOOKUP(+1xxxxxxxxxx,ALL,4,e164.org)});
};


   -- Executing NoOp("SIP/2301-a102", "iax2:guest@iax2.voip.dashjr.org/1xxxxxxxxxx") in new stack
   -- Executing NoOp("SIP/2301-a102", "iax2:guest@iax2.voip.dashjr.org/1xxxxxxxxxx") in new stack
   -- Executing NoOp("SIP/2301-a102", "MAILTO:dashjrs_1xxxxxxxxxx@dashjr.org") in new stack
   -- Executing NoOp("SIP/2301-a102", "ADDRESS:CN=Luke and Corinne -Jr;STREET=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx;L=Riverside;ST=Missouri;C=United States") in new stack
(the second line should be SIP)


as for newer versions, it's installing now...
By: Tilghman Lesher (tilghman) 2006-07-13 15:39:34

Since there hasn't been any change to the enum code since before 1.2.7, there's no need to retest this.

However, this isn't strictly a bug, either.  The issue is this:  each time you call ENUMLOOKUP, we do a separate query for the results (i.e. it's not cached), but each time that you've done the query, the results are returned in a slightly different order.  While we automatically sort the entries to try to get the right order, in this case, having the same sort numbers, we have no way of knowing how previous listings were sorted.

While I could use the tech to indicate a 3rd criteria upon which to sort, I think that would override the purpose of you setting these to the same priority, which would seem to be for load balancing purposes.

So this isn't really a bug, but I agree that it's undesireable behavior.  The question then becomes is this something we want to fix in Asterisk, or should we require you to set unique priorities, if you want the sorting to always come back exactly the same way?  I'm leaning towards the second option.
By: Luke-Jr (luke-jr) 2006-07-13 16:03:15

I would consider it a bug even if simply because it is impossible to get the second element under any circumstances. If the sort was predictable (eg, maybe a simple strcmp on the value), at least you could iterate over the results.

There's no load balancing in effect-- I just don't have a preference for either IAX2 or SIP... While in practice, I have simply sorted the two separately, it is still a bug to require this since the relevant RFCs don't forbid multiple entries with the same order (and in fact have examples with such cases).
By: Tilghman Lesher (tilghman) 2006-07-13 17:26:42

Please try this patch and report your results.
By: Luke-Jr (luke-jr) 2006-07-13 19:15:18

Any chance you can make a patch that doesn't involve a bunch of syntax reformatting? (diff -Bb helps, but there's other bits in there)
By: Tilghman Lesher (tilghman) 2006-07-14 00:07:02

You shouldn't need to strip those changes to apply that patch.  Just cd to your asterisk 1.2 directory and use the patch command to apply it:

patch -p0 < /path/to/patch
By: Tilghman Lesher (tilghman) 2006-07-15 18:38:05

Fixed in 1.2; merged to trunk.