[Home]

Summary:ASTERISK-14409: [patch] URIENCODE() throws a warning when passed an empty string
Reporter:pkempgen (pkempgen)Labels:
Date Opened:2009-07-02 00:54:08Date Closed:2009-07-21 17:49:05
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Functions/func_uri
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20090706__issue15439.diff.txt
Description:URIENCODE(${empty}) throws a warning:
WARNING[23535]: func_uri.c:51 uriencode: Syntax: URIENCODE(<data>) - missing argument!
other functions like
QUOTE(${empty})
don't complain.

****** ADDITIONAL INFORMATION ******

---cut---
[Syntax]
URIENCODE(<data>)
---cut---

---cut---
[Syntax]
QUOTE(<string>)
---cut---
Comments:By: Tilghman Lesher (tilghman) 2009-07-06 17:53:43

Given the syntax (argument NOT optional), QUOTE should complain with a warning.  Are you really sure you want to pursue this?


By: Jason Parker (jparker) 2009-07-06 18:01:16

Devil's advocate: It could be argued that the documentation is flawed as well.

Edit: I don't have an opinion either way.


By: Sean Bright (seanbright) 2009-07-06 19:05:19

It is perfectly valid to try and quote or url encode/decode an empty string.  Also, as there is no way to distinguish between no argument and an empty argument, I'd lean towards just accepting an empty argument without warning.  Either way this particular issue doesn't deserve the time being dedicated to it.
By: Tilghman Lesher (tilghman) 2009-07-06 20:08:28

qwell:  while I've suggested that approach in the past, I get overridden by the documentation folks.  Since then, the approach has been to follow the documentation scrupulously, unless the documentation can be shown to be error-prone (in other words, do something that is really bad).
By: Tilghman Lesher (tilghman) 2009-07-06 20:18:07

seanbright: if you want to spend the time to get a clarification from the asterisk-doc project, feel free.
By: Sean Bright (seanbright) 2009-07-06 20:27:04

I've uploaded a different solution in the attached patch (20090706_issue15439_alternate.diff)


By: Tilghman Lesher (tilghman) 2009-07-07 01:34:51

seanbright:  you don't need to zero the buffer on error.  Just return -1 and the API will do it for you.
By: Sean Bright (seanbright) 2009-07-07 05:11:43

Right, only it's not an error, which is why I am zeroing out the buffer.

Edit: Well, now that I actually read your patch, I think it's fine.  I do think that changing the warning message to something like "Empty string or no argument specified" would be more clear, however.


By: pkempgen (pkempgen) 2009-07-12 13:18:45

IMO an empty string is still a valid argument
(the fact that strings are not quoted is a different story)
and thus 'QUOTE()' should evaluate to '""' and
'URIENCODE()' should evaluate to '' (single quotes added
here for clarification).
However I just wanted to point out that there is a difference
in the behavior of these 2 functions even though the syntax in
the documentation is identical (apart from the function name
of course).
Do whatever you like best. It doesn't deserve more time
being dedicated to it.
By: Digium Subversion (svnbot) 2009-07-21 17:38:57

Repository: asterisk
Revision: 207945

U   branches/1.4/funcs/func_strings.c

------------------------------------------------------------------------
r207945 | tilghman | 2009-07-21 17:38:57 -0500 (Tue, 21 Jul 2009) | 8 lines

Force an error if a blank is passed to QUOTE (because the documentation states the argument is not optional).
This change makes URIENCODE and QUOTE behave similarly, since the documentation
states that the argument is not optional, for both.
(closes issue ASTERISK-14409)
Reported by: pkempgen
Patches:
      20090706__issue15439.diff.txt uploaded by tilghman (license 14)

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

http://svn.digium.com/view/asterisk?view=rev&revision=207945
By: Digium Subversion (svnbot) 2009-07-21 17:45:35

Repository: asterisk
Revision: 207946

_U  trunk/
U   trunk/funcs/func_strings.c

------------------------------------------------------------------------
r207946 | tilghman | 2009-07-21 17:45:35 -0500 (Tue, 21 Jul 2009) | 15 lines

Merged revisions 207945 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r207945 | tilghman | 2009-07-21 17:38:54 -0500 (Tue, 21 Jul 2009) | 8 lines
 
 Force an error if a blank is passed to QUOTE (because the documentation states the argument is not optional).
 This change makes URIENCODE and QUOTE behave similarly, since the documentation
 states that the argument is not optional, for both.
 (closes issue ASTERISK-14409)
  Reported by: pkempgen
  Patches:
        20090706__issue15439.diff.txt uploaded by tilghman (license 14)
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=207946
By: Digium Subversion (svnbot) 2009-07-21 17:47:34

Repository: asterisk
Revision: 207947

_U  branches/1.6.0/
U   branches/1.6.0/funcs/func_strings.c

------------------------------------------------------------------------
r207947 | tilghman | 2009-07-21 17:47:34 -0500 (Tue, 21 Jul 2009) | 22 lines

Merged revisions 207946 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r207946 | tilghman | 2009-07-21 17:45:32 -0500 (Tue, 21 Jul 2009) | 15 lines
 
 Merged revisions 207945 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r207945 | tilghman | 2009-07-21 17:38:54 -0500 (Tue, 21 Jul 2009) | 8 lines
   
   Force an error if a blank is passed to QUOTE (because the documentation states the argument is not optional).
   This change makes URIENCODE and QUOTE behave similarly, since the documentation
   states that the argument is not optional, for both.
   (closes issue ASTERISK-14409)
    Reported by: pkempgen
    Patches:
          20090706__issue15439.diff.txt uploaded by tilghman (license 14)
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=207947
By: Digium Subversion (svnbot) 2009-07-21 17:48:56

Repository: asterisk
Revision: 207948

_U  branches/1.6.1/
U   branches/1.6.1/funcs/func_strings.c

------------------------------------------------------------------------
r207948 | tilghman | 2009-07-21 17:48:56 -0500 (Tue, 21 Jul 2009) | 22 lines

Merged revisions 207946 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r207946 | tilghman | 2009-07-21 17:45:32 -0500 (Tue, 21 Jul 2009) | 15 lines
 
 Merged revisions 207945 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r207945 | tilghman | 2009-07-21 17:38:54 -0500 (Tue, 21 Jul 2009) | 8 lines
   
   Force an error if a blank is passed to QUOTE (because the documentation states the argument is not optional).
   This change makes URIENCODE and QUOTE behave similarly, since the documentation
   states that the argument is not optional, for both.
   (closes issue ASTERISK-14409)
    Reported by: pkempgen
    Patches:
          20090706__issue15439.diff.txt uploaded by tilghman (license 14)
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=207948
By: Digium Subversion (svnbot) 2009-07-21 17:49:04

Repository: asterisk
Revision: 207949

_U  branches/1.6.2/
U   branches/1.6.2/funcs/func_strings.c

------------------------------------------------------------------------
r207949 | tilghman | 2009-07-21 17:49:03 -0500 (Tue, 21 Jul 2009) | 22 lines

Merged revisions 207946 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r207946 | tilghman | 2009-07-21 17:45:32 -0500 (Tue, 21 Jul 2009) | 15 lines
 
 Merged revisions 207945 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r207945 | tilghman | 2009-07-21 17:38:54 -0500 (Tue, 21 Jul 2009) | 8 lines
   
   Force an error if a blank is passed to QUOTE (because the documentation states the argument is not optional).
   This change makes URIENCODE and QUOTE behave similarly, since the documentation
   states that the argument is not optional, for both.
   (closes issue ASTERISK-14409)
    Reported by: pkempgen
    Patches:
          20090706__issue15439.diff.txt uploaded by tilghman (license 14)
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=207949