#15 DLINK DIR-625 patch
Basically, UPNPSendAction() fails with -113 (UPNP_E_BAD_RESPONSE) when trying to read a simple HTML response from a DLINK DIR-625 because the 625 is using "Chunked" Transfer Encoding, when really it sends it all in one sweep anyway. The switch from a strstr() call to strcasestr is because their firmware sends out a tag that matches, but only case-insensitively.
The patch detects and rewrites the recieved buffer, non-chunked style and resets a few variables so that the rest of the parsing engine can't tell the difference.
This patch was written for the DLINK DIR-625 (Hardware: A1, firmware: 1.0.9) but may fix communication problems with other devices as well.
I would have preferred to modify the entire parser to accomodate such rotten firmware, but it was much simpler to write this patch than try to fully understand and modify the parser.
I send an email to DLINK regarding this problem and did not recieve a response.
Please let me know if you need more information.
Craig Nelson, Cameo Systems Inc.
Logged In: YES
user_id=67568
Originator: NO
Craig,
Please do not do that again. You have already opened an issue with the same subject here:
https://sourceforge.net/tracker/index.php?func=detail&aid=1688930&group_id=166957&atid=841028
The issue has been closed by the source forge robot because you have not answered. If you want, you can reopen it.
Regards,
Marcelo.
patches libupnp-1.4.1 thru 1.4.3
Logged In: YES
user_id=1754020
Originator: YES
Marcelo,
Thanks for re-opening this issue. I've included an updated patch file (libupnp-dlink-dir625.patch) which contains comments as well as other minor modifications required for a clean compilation (no compiler warnings) and a patch segment that I had failed to include the first time. The previous patch submission has been deleted as this patch supercedes it.
An explanation of the patch follows:
Here is what happens without the patch: UPNPSendAction() fails with -113 (UPNP_E_BAD_RESPONSE) when trying to read a simple HTML response from a DLINK DIR-625 because the 625 says it is using "Chunked" Transfer Encoding. However, the entire result is in the first chunk anyway. The switch from a call to strstr() call to strcasestr() is because the 625 firmware sends out a tag that matches, but only case-insensitively.
When an incorrect buffer comes in, the patch rewrites it, non-chunked style and resets a few (parser) variables so that the rest of the parsing engine can't tell the difference and will continue to parse the information.
This patch was written for the DLINK DIR-625 (Hardware: A1, firmware: 1.0.9). Anyone using this device will not be able to communicate correctly with libupn
p and can safely use this patch. As well, this patch may fix communication problems with other devices. Again, I have tested this patch with a number of other firewall/routers - it does not interfere with communcations from those devices.
Please let me know if you need more information. I would be glad to assist anyone in making the proper changes to libupnp, but I'll need some help from someo
ne who wrote the original code :)
Patch changes include:
FILES: ixml/inc/ixml.h, upnp/src/genlib/net/http/httpparser.c:
- Prevent compilation warning: don't declare a variable name that is also a typedef. renamed variable declarations to include the word "this". ie: this_IXML_Node, this_index
- Change call from strstr() to strcasestr() due to case insensitivity described above
FILE: upnp/src/genlib/net/http/httpreadwrite.c
- Added dlink_dir625_patch() function to detect missing HTTP header information that the rest of libupnp relies on to
communicate with a remote device. Without a call to this function, UPNPSendAction() returns -113: UPNP_E_BAD_RESPONSE because it
expects to receive chunked information that is not actually going to be sent by the device. It would be much cleaner to scratch this patch altogether and make the original source code more forgiving when dealing with missing pieces of information. There may be other routers that do the same thing.
See patch file for additional comments.
Craig Nelson, Cameo Systems Inc.
To apply this patch: cd libupnp-1.4.3 ; patch -p1 < dlink-dir625.patch
File Added: dlink-dir625.patch
Logged In: YES
user_id=67568
Originator: NO
Hi Craig,
Sorry for taking so long to answer.
Could you please tell me what compiler and what system you are using? I get no warning message here (linux/gcc 4.1.3) for the declaration of "IXML_Node *IXML_Node". Anyway, I agree it is ugly, so I have changed it.
But I think it is very strange that your compiler warns about "unsigned long index". "index" is not a typedef, could you tell me what is the warning your compiler gives you?
I have accepted your change to httpparser.c, in the current code in the subversion repository raw_find_str() now uses strcasestr() to compare strings.
That leaves us with dlink_dir625_hack() only now. I see you fixed the memleaks. But still, I cannot accept such a hack into the code. My suggestion is that we work towards fixing the problem in the parser, as you said, "make it more forgiving". I believe that you have plenty of conditions to do so. And you have the hardware to test it. Notice that I am not the original author of the code, and I would have to study it in order to "do the right thing" at the right place, and I currently do not have the necessary time resources. On the other hand, if you "do the right thing", I will accept your patch gladly.
One other issue, you are producing your patches against an old version of the library, and they don't apply cleanly to the present code. Please, if you want to contribute with patches, the easiest way is to check out a copy of the source code of the library using source forge's subversion repository. The command is:
$ svn co https://pupnp.svn.sourceforge.net/svnroot/pupnp/trunk libupnp-trunk
This will create a directory called libupnp-trunk that will have the full current source code of the library. You can easily produce patches doing:
$ svn diff > p1.patch
Or see what you have done:
$ svn diff | less
Thank you for your cooperation, and I hope you decide to go on and really fix the parser. If I can be of any help, please do not hesitate in contacting me.
Best regards,
Marcelo.
Logged In: YES
user_id=1754020
Originator: YES
Marcelo,
I'm sorry but I am unable to take on the task of patching the parser myself as I do not have the time either. but to answer your question, I was getting those compiler warnings when compiling with cygwin/gcc. I agree they are just simple warnings and I'm glad to see you accepted those changes. Thanks for the tips on svn.. that's another package I intend on learning and installing soon. Please keep the patch file available as I'm sure someone will eventually need it. Hopefully I'll get some time in the future to fix the parser, but now is not a good time..
Craig