UPnPPortMapper AccessViolation it is cause by MiniUPnP.UPNP_GetIGDFromUrl !!!

[GoogleCodeExporter]

What version of the product are you using? On what operating system?
Vista pro edition - AMD Turion64

The problem:

In: TCUPnPPortMapper.cs  Procedure: RefreshExternalIPThread()
The call of "MiniUPnP.UPNP_GetIGDFromUrl(url, ref urls, ref igddata, 
lanAddr, lanAddr.Length)" corrupt the program memory.
The each time you use "urls" you have an access violation.

Why: If you watch in miniupnpc.c GetIGDFromUrl call GetUPNPUrls witch make 
malloc on urls. C# seem to not like that.
Probably "urls" is not well declared but i can't find the good way to make 
it run.
Be aware that a malloc is done so after use you must call FreeUPNPUrls
(struct UPNPUrls * urls) in the dll!!!

Solution I've tested its works (no more accessviolation....)

-> In Class MiniUPnP (MiniUPnP.cs) i've created:

[StructLayout(LayoutKind.Sequential)]
        public unsafe struct UPNPUrls_2
        {
            public char *controlURL;

            public char *ipcondescURL;

            public char* controlURL_CIF;
        }

and 

[DllImport("miniupnp.dll", CallingConvention = CallingConvention.Cdecl)]
        public unsafe static extern void FreeUPNPUrls([In] UPNPUrls_2* 
urls);

and add a new declaration of

[DllImport("miniupnp.dll", CallingConvention = CallingConvention.Cdecl)]
public static unsafe extern int UPNP_GetIGDFromUrl
       ([In] String rootDescUrl,
        [In] UPNPUrls_2 *urls,
        [In, Out] ref IGDdatas datas,
        [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 4)] byte[] 
lanAddr,
        [In] int lanAddrLength);

-> In TCUPnPPortMapper.cs:
 Relace the call of MiniUPnP.UPNP_GetIGDFromUrl by
try
 {
    //r = MiniUPnP.UPNP_GetIGDFromUrl(url, ref urls, ref igddata, lanAddr, 
lanAddr.Length);
    MiniUPnP.UPNPUrls_2 urls_2 = new MiniUPnP.UPNPUrls_2();
    unsafe
      {
        r = MiniUPnP.UPNP_GetIGDFromUrl(url, &urls_2, ref igddata, 
lanAddr, 
lanAddr.Length);                                                       
        MiniUPnP.FreeUPNPUrls(&urls_2);                           
      }

     // Renseigne urls
     GetUPNPUrls(url); // On se subsitue a miniUpnp pour renseigner 
urls                     

}

Then create GetUPNPUrls (wich replace what it made in dll)

        private void GetUPNPUrls(String url)
        {
            // Copie de l'URL de Base
            if (igddata.urlbase != string.Empty)
                urls.ipcondescURL = igddata.urlbase;
            else
                urls.ipcondescURL = url;

            // On enleve tout apres l'URL de base
            int index_fin_url = urls.ipcondescURL.IndexOf('/', 7); // 7 = 
http://
            if (index_fin_url >= 0)
                urls.ipcondescURL = urls.ipcondescURL.Substring(0, 
index_fin_url);

            urls.controlURL = urls.ipcondescURL + igddata.controlurl;
            urls.controlURL_CIF = urls.ipcondescURL + 
igddata.controlurl_CIF;
            urls.ipcondescURL += igddata.scpdurl;
        }

---------------------------------------------------

This solution is not very nice but it works.
I think the best way of correct the bug is to redeclare UPNPUrls but i 
can't find how to do that.
An other way is to modify the miniupnp.dll (remove the malloc) and create 
the function GetUPNPUrls int the c# code

Let me know if you find something better !!!!!

Original issue reported on code.google.com by jfkot...@gmail.com on 23 Jan 2009 at 10:58

[GoogleCodeExporter]

I'm working on integrating this change into the project.  Was this bug causing 
the 
application to crash?

Original comment by robbieha...@gtempaccount.com on 3 Apr 2009 at 11:19

[GoogleCodeExporter]

Yes, that's why there are try...catch in the code each time the dll is called.

Original comment by jfkot...@gmail.com on 4 Apr 2009 at 9:17

[GoogleCodeExporter]

Thank you very much for this patch!

Your changes have been committed.  Sorry it took us forever to do it!

Original comment by deu...@gmail.com on 6 Apr 2009 at 10:00

• Changed state: Fixed