Review Board 1.7.16


ast_uri_encode() behavior change

Review Request #451 - Created Dec. 23, 2009 and discarded

David Vossel
trunk
16299
Reviewers
asterisk-dev
Asterisk
This patch changes ast_uri_encode()'s behavior when doreserved is enabled.  Previously when doreserved was enabled only a small set of reserved characters were encoded.  This set was comprised primarily of the reserved characters defined in RFC3261 section 25.1, but contained other characters as well.  Rather than only escaping the reserved set, doreserved now escapes all characters not within the unreserved set as defined by RFC 3261 and RFC 2396.  Also, the 'doreserved' variable has been renamed to 'do_special_char' in attempts to avoid confusion.

When doreserve is not enabled, the previous logic of only encoding the characters <= 0X1F and > 0X7f remains.

In RFC 3261 and RFC 2396 the unreserved character set is defined by all alphanumeric characters and a small number of characters defined in the mark set.
mark        =  "-" / "_" / "." / "!" / "~" / "*" / "'"
                     / "(" / ")"
unreserved  =  alphanum / mark


I have viewed several discussions involving this code lately.  My hopes are that this patch will prompt continued discussion and help bring us to a solution we can all agree upon. 

This patch contains logic written by both wdoekes and myself.  Discussions pertaining to this issue may either be discussed here or directed to (issue #16299).
I wrote a Unit Test to verify ast_uri_encode's output.  The results are below.

START  main/utils/ - uri_encode_test 

Input before executing ast_uri_encode:
abcdefghijklmnopurstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ~`!@#$%^&*()_-+={[}]|\:;"'<,>.?/
Output expected for ast_uri_encode with enabling do_special_char:
abcdefghijklmnopurstuvwxyz%20ABCDEFGHIJKLMNOPQRSTUVWXYZ%201234567890%20~%60!%40%23%24%25%5e%26*()_-%2b%3d%7b%5b%7d%5d%7c%5c%3a%3b%22'%3c%2c%3e.%3f%2f

Output after enabling do_special_char:
abcdefghijklmnopurstuvwxyz%20ABCDEFGHIJKLMNOPQRSTUVWXYZ%201234567890%20~%60!%40%23%24%25%5e%26*()_-%2b%3d%7b%5b%7d%5d%7c%5c%3a%3b%22'%3c%2c%3e.%3f%2f
Decoded string matched original input

Output after disabling do_special_char:
abcdefghijklmnopurstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ~`!@#$%^&*()_-+={[}]|\:;"'<,>.?/
Decoded string matched original input

END    main/utils/ - uri_encode_test Time: 0ms Result: PASS 

Diff revision 2 (Latest)

1 2
1 2

  1. /trunk/channels/chan_sip.c: Loading...
  2. /trunk/include/asterisk/utils.h: Loading...
  3. /trunk/main/utils.c: Loading...
/trunk/channels/chan_sip.c
Revision 236309 New Change
[20] 266 lines
[+20] [+] ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
267
#include "asterisk/event.h"
267
#include "asterisk/event.h"
268
#include "asterisk/tcptls.h"
268
#include "asterisk/tcptls.h"
269
#include "asterisk/stun.h"
269
#include "asterisk/stun.h"
270
#include "asterisk/cel.h"
270
#include "asterisk/cel.h"
271
#include "asterisk/strings.h"
271
#include "asterisk/strings.h"

    
   
272
#include "asterisk/test.h"
272

    
   
273

   
273
/*** DOCUMENTATION
274
/*** DOCUMENTATION
274
	<application name="SIPDtmfMode" language="en_US">
275
	<application name="SIPDtmfMode" language="en_US">
275
		<synopsis>
276
		<synopsis>
276
			Change the dtmfmode for a SIP call.
277
			Change the dtmfmode for a SIP call.
[+20] [20] 26596 lines
[+20] [+] static struct ast_cli_entry cli_sip[] = {
26873
	AST_CLI_DEFINE(sip_set_history, "Enable/Disable SIP history"),
26874
	AST_CLI_DEFINE(sip_set_history, "Enable/Disable SIP history"),
26874
	AST_CLI_DEFINE(sip_reload, "Reload SIP configuration"),
26875
	AST_CLI_DEFINE(sip_reload, "Reload SIP configuration"),
26875
	AST_CLI_DEFINE(sip_show_tcp, "List TCP Connections")
26876
	AST_CLI_DEFINE(sip_show_tcp, "List TCP Connections")
26876
};
26877
};
26877

    
   
26878

   

    
   
26879
/* DEFINE SIP TESTS HERE */

    
   
26880
AST_TEST_DEFINE(sip_encode_decode_test)

    
   
26881
{

    
   
26882
	int res = AST_TEST_PASS;

    
   
26883
	const char *in = "abcdefghijklmnopurstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ~`!@#$%^&*()_-+={[}]|\\:;\"'<,>.?/";

    
   
26884
	const char *expected1 = "abcdefghijklmnopurstuvwxyz%20ABCDEFGHIJKLMNOPQRSTUVWXYZ%201234567890%20~%60!%40%23%24%25%5E%26*()_-%2B%3D%7B%5B%7D%5D%7C%5C%3A%3B%22'%3C%2C%3E.%3F%2F";

    
   
26885
	const char *expected2 = "abcdefghijklmnopurstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ~`!@#$%25^&*()_-+={[}]|\\:;\"'<,>.?/";

    
   
26886
	char out[256] = { 0 };

    
   
26887

   

    
   
26888
	switch (cmd) {

    
   
26889
	case TEST_INIT:

    
   
26890
		info->name = "uri_encode_test";

    
   
26891
		info->category = "main/utils/";

    
   
26892
		info->summary = "encode a string";

    
   
26893
		info->description = "encode a string, verify encoded string matches what we expect.";

    
   
26894
		return AST_TEST_NOT_RUN;

    
   
26895
	case TEST_EXECUTE:

    
   
26896
		break;

    
   
26897
	}

    
   
26898

   

    
   
26899
	ast_test_status_update(&args->status_update, "Input before executing ast_uri_encode:\n%s\n", in);

    
   
26900
	ast_test_status_update(&args->status_update, "Output expected for ast_uri_encode with enabling do_special_char:\n%s\n", expected1);

    
   
26901
	ast_test_status_update(&args->status_update, "Output expected for ast_uri_encode with out enabling do_special_char:\n%s\n\n", expected2);

    
   
26902

   

    
   
26903
	/* Test with do_special_char enabled, no force_no_encode set */

    
   
26904
	ast_uri_encode(in, out, sizeof(out), 1);

    
   
26905
	ast_test_status_update(&args->status_update, "Output after enabling do_special_char:\n%s\n", out);

    
   
26906
	if (strcmp(expected1, out)) {

    
   
26907
		ast_test_status_update(&args->status_update, "ENCODE DOES NOT MATCH EXPECTED, FAIL\n");

    
   
26908
		res = AST_TEST_FAIL;

    
   
26909
	}

    
   
26910
	ast_uri_decode(out);

    
   
26911
	if (strcmp(in, out)) {

    
   
26912
		ast_test_status_update(&args->status_update, "Decoded string did not match original input\n\n");

    
   
26913
		res = AST_TEST_FAIL;

    
   
26914
	} else {

    
   
26915
		ast_test_status_update(&args->status_update, "Decoded string matched original input\n\n");

    
   
26916
	}

    
   
26917

   

    
   
26918
	/* Test with do_special_char disabled */

    
   
26919
	out[0] = '\0';

    
   
26920
	ast_uri_encode(in, out, sizeof(out), 0);

    
   
26921
	ast_test_status_update(&args->status_update, "Output after disabling do_special_char:\n%s\n", out);

    
   
26922
	if (strcmp(expected2, out)) {

    
   
26923
		ast_test_status_update(&args->status_update, "ENCODE DOES NOT MATCH EXPECTED, FAIL\n");

    
   
26924
		res = AST_TEST_FAIL;

    
   
26925
	}

    
   
26926

   

    
   
26927
	ast_uri_decode(out);

    
   
26928
	if (strcmp(in, out)) {

    
   
26929
		ast_test_status_update(&args->status_update, "Decoded string did not match original input\n\n");

    
   
26930
		res = AST_TEST_FAIL;

    
   
26931
	} else {

    
   
26932
		ast_test_status_update(&args->status_update, "Decoded string matched original input\n\n");

    
   
26933
	}

    
   
26934
	return res;

    
   
26935
}

    
   
26936

   

    
   
26937
static void sip_register_tests(void)

    
   
26938
{

    
   
26939
	AST_TEST_REGISTER(sip_encode_decode_test);

    
   
26940
}

    
   
26941

   

    
   
26942
static void sip_unregister_tests(void)

    
   
26943
{

    
   
26944
	AST_TEST_UNREGISTER(sip_encode_decode_test);

    
   
26945
}

    
   
26946

   

    
   
26947

   
26878
/*! \brief PBX load module - initialization */
26948
/*! \brief PBX load module - initialization */
26879
static int load_module(void)
26949
static int load_module(void)
26880
{
26950
{
26881
	ast_verbose("SIP channel loading...\n");
26951
	ast_verbose("SIP channel loading...\n");
26882
	/* the fact that ao2_containers can't resize automatically is a major worry! */
26952
	/* the fact that ao2_containers can't resize automatically is a major worry! */
[+20] [20] 80 lines
[+20] static int load_module(void)
26963
		"regserver", RQ_CHAR, 20,
27033
		"regserver", RQ_CHAR, 20,
26964
		"useragent", RQ_CHAR, 20,
27034
		"useragent", RQ_CHAR, 20,
26965
		"lastms", RQ_INTEGER4, 11,
27035
		"lastms", RQ_INTEGER4, 11,
26966
		SENTINEL);
27036
		SENTINEL);
26967

    
   
27037

   

    
   
27038
	sip_register_tests();

    
   
27039

   
26968
	return AST_MODULE_LOAD_SUCCESS;
27040
	return AST_MODULE_LOAD_SUCCESS;
26969
}
27041
}
26970

    
   
27042

   
26971
/*! \brief PBX unload module API */
27043
/*! \brief PBX unload module API */
26972
static int unload_module(void)
27044
static int unload_module(void)
[+20] [20] 112 lines
[+20] static int load_module(void)
27085
	if (con)
27157
	if (con)
27086
		ast_context_destroy(con, "SIP");
27158
		ast_context_destroy(con, "SIP");
27087
	ast_unload_realtime("sipregs");
27159
	ast_unload_realtime("sipregs");
27088
	ast_unload_realtime("sippeers");
27160
	ast_unload_realtime("sippeers");
27089

    
   
27161

   

    
   
27162
	sip_unregister_tests();

    
   
27163

   
27090
	return 0;
27164
	return 0;
27091
}
27165
}
27092

    
   
27166

   
27093
AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT, "Session Initiation Protocol (SIP)",
27167
AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT, "Session Initiation Protocol (SIP)",
27094
		.load = load_module,
27168
		.load = load_module,
27095
		.unload = unload_module,
27169
		.unload = unload_module,
27096
		.reload = reload,
27170
		.reload = reload,
27097
	       );
27171
	       );
/trunk/include/asterisk/utils.h
Revision 236309 New Change
 
/trunk/main/utils.c
Revision 236309 New Change
 
  1. /trunk/channels/chan_sip.c: Loading...
  2. /trunk/include/asterisk/utils.h: Loading...
  3. /trunk/main/utils.c: Loading...

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.