| Summary: | ASTERISK-14952: [patch] AGI returns bogus "510 Invalid or unknown command" | ||
| Reporter: | Pete Yandell (notahat) | Labels: | |
| Date Opened: | 2009-10-06 21:19:23 | Date Closed: | 2011-05-18 09:41:28 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | Resources/res_agi |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) agi_buffer_patch.diff ( 1) agi_buffer_patch2.diff ( 2) agi_log.txt | |
| Description: | When res_agi.c reads from the AGI stream, it incorrectly calculates the number of free bytes remaining in the read buffer. In certain circumstances (such as repeated EAGAIN errors from fgets), this causes it to incorrectly interpret the buffer as full. That then confuses the code interpreting the command, leading to a bogus "510 invalid or unknown command" error being returned. ****** ADDITIONAL INFORMATION ****** Inside the run_agi function, several variables are used to keep track of the buffer: - buf is the buffer itself - len is the remaining free space in the buffer - buflen is the number of bytes currently in the buffer Each time through the read loop, buflen is subtracted from len (line 2727 of res_agi.c). Note that buflen is the total number of bytes in the buffer, not just the bytes that were read on this pass of the loop. fgets can return repeated EAGAIN errors, resulting in the read loop running many times. Each time through, len is reduced by the total number of bytes in the buffer, despite no new data being read. Eventually the buffer is interpreted (incorrectly) as empty. Attached is an AGI debug log showing the problem, and a patch. | ||
| Comments: | By: Marcus Hunger (fnordian) 2010-10-18 05:55:15 i can confirm this bug. it also triggers when the agi sends more data in one line then fgets is able to return at once. the while-condition should also be fixed. unless i miss something, it should only care about how much space is left in the target-buffer. By: Digium Subversion (svnbot) 2011-01-19 12:37:11.000-0600 Repository: asterisk Revision: 302548 U branches/1.6.2/res/res_agi.c ------------------------------------------------------------------------ r302548 | seanbright | 2011-01-19 12:37:10 -0600 (Wed, 19 Jan 2011) | 10 lines Properly handle partial reads from fgets() when handling AGIs. When fgets() failed with EAGAIN, we were continually decrementing the available space left in our buffer, resulting in botched command handling. (closes issue ASTERISK-14952) Reported by: notahat Patches: agi_buffer_patch2.diff uploaded by fnordian (license 110) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=302548 By: Digium Subversion (svnbot) 2011-01-19 12:43:12.000-0600 Repository: asterisk Revision: 302549 _U branches/1.8/ U branches/1.8/res/res_agi.c ------------------------------------------------------------------------ r302549 | seanbright | 2011-01-19 12:43:12 -0600 (Wed, 19 Jan 2011) | 17 lines Merged revisions 302548 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ........ r302548 | seanbright | 2011-01-19 13:37:09 -0500 (Wed, 19 Jan 2011) | 10 lines Properly handle partial reads from fgets() when handling AGIs. When fgets() failed with EAGAIN, we were continually decrementing the available space left in our buffer, resulting in botched command handling. (closes issue ASTERISK-14952) Reported by: notahat Patches: agi_buffer_patch2.diff uploaded by fnordian (license 110) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=302549 By: Digium Subversion (svnbot) 2011-01-19 12:45:46.000-0600 Repository: asterisk Revision: 302550 _U trunk/ U trunk/res/res_agi.c ------------------------------------------------------------------------ r302550 | seanbright | 2011-01-19 12:45:45 -0600 (Wed, 19 Jan 2011) | 24 lines Merged revisions 302549 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ................ r302549 | seanbright | 2011-01-19 13:43:11 -0500 (Wed, 19 Jan 2011) | 17 lines Merged revisions 302548 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ........ r302548 | seanbright | 2011-01-19 13:37:09 -0500 (Wed, 19 Jan 2011) | 10 lines Properly handle partial reads from fgets() when handling AGIs. When fgets() failed with EAGAIN, we were continually decrementing the available space left in our buffer, resulting in botched command handling. (closes issue ASTERISK-14952) Reported by: notahat Patches: agi_buffer_patch2.diff uploaded by fnordian (license 110) ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=302550 | ||