calejost / unimrcp

Automatically exported from code.google.com/p/unimrcp
Apache License 2.0
0 stars 0 forks source link

Some MRCP servers "lie" about "existing connections." #41

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What version of the product is patch made on?
unimrcp-0.7.0

Please use labels and text to provide additional information.

Certain MRCP servers indicate that they want to re-use an existing
connection, but offer up a different port number for the "existing"
connection.  This creates a situation where the code doesn't examine the
port number and tries to use the original port number instead of the new
one.  This patch will ignore the "existing" tag if the new port number is
not the same as the current one.  Should the patch also check to make sure
the offered port number is not 0 as well in case it is not specified?

Diff of: mrcp_client_connection.c 

< = New
> = Existing

371c371
<                       if(!connection || connection->r_sockaddr->port !=
descriptor->port) {

---
>                       if(!connection) {

Original issue reported on code.google.com by asackh...@gmail.com on 9 Sep 2009 at 3:44

GoogleCodeExporter commented 8 years ago
I agree, we should be tolerant against issues like that. Actually I considered 
this
issue had been already addressed.
At least code tries to find an existing connection first, see
mrcp_client_agent_connection_find(), if found uses it, otherwise creates new
connection, even if "existing" is specified in answer from server. However as it
turns out apr_sockaddr_equal() matches only IP addresses, but ports indeed must 
be
matched too.

Original comment by achalo...@gmail.com on 9 Sep 2009 at 6:29

GoogleCodeExporter commented 8 years ago
My patch was just a "serving suggestion".  If you have a better place that is 
more
generic to patch, feel free.  This was the simplest way for me to understand 
what was
going on and fix it.

Original comment by asackh...@gmail.com on 9 Sep 2009 at 6:35

GoogleCodeExporter commented 8 years ago
Well, I've just fixed it in r1111
http://code.google.com/p/unimrcp/source/detail?r=1111

Please give it a try and let me know if it works for you.

Original comment by achalo...@gmail.com on 9 Sep 2009 at 6:48

GoogleCodeExporter commented 8 years ago
I made the patch as you suggested and it works just fine.  Bravo!

Original comment by asackh...@gmail.com on 9 Sep 2009 at 8:58

GoogleCodeExporter commented 8 years ago

Original comment by achalo...@gmail.com on 5 Oct 2009 at 4:28