From c0cfa2eedaebe7e02110f9d0b2f54f7d04759130 Mon Sep 17 00:00:00 2001 From: Chow Loong Jin Date: Thu, 14 May 2015 10:11:24 +0800 Subject: [PATCH 1/4] Fix out of bounds array access when unable to find pattern --- django_js_reverse/templates/django_js_reverse/urls_js.tpl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/django_js_reverse/templates/django_js_reverse/urls_js.tpl b/django_js_reverse/templates/django_js_reverse/urls_js.tpl index c1daafa..a06748e 100644 --- a/django_js_reverse/templates/django_js_reverse/urls_js.tpl +++ b/django_js_reverse/templates/django_js_reverse/urls_js.tpl @@ -11,7 +11,8 @@ var index, url, url_arg, url_args, _i, _len, _ref, _ref_list; _ref_list = self.url_patterns[url_pattern]; for (_i = 0; - _ref = _ref_list[_i], _ref[1].length != arguments.length; + _i < _ref_list.length && + (_ref = _ref_list[_i], _ref[1].length != arguments.length); _i++); url = _ref[0], url_args = _ref[1]; From 16a9a28e3db7867f15c57795050bca595633b460 Mon Sep 17 00:00:00 2001 From: Chow Loong Jin Date: Thu, 14 May 2015 16:35:01 +0800 Subject: [PATCH 2/4] Implement key-based url reversing --- .../templates/django_js_reverse/urls_js.tpl | 62 ++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/django_js_reverse/templates/django_js_reverse/urls_js.tpl b/django_js_reverse/templates/django_js_reverse/urls_js.tpl index a06748e..699fad2 100644 --- a/django_js_reverse/templates/django_js_reverse/urls_js.tpl +++ b/django_js_reverse/templates/django_js_reverse/urls_js.tpl @@ -8,17 +8,65 @@ var _get_url = function (url_pattern) { return function () { - var index, url, url_arg, url_args, _i, _len, _ref, _ref_list; + var _arguments, index, url, url_arg, url_args, _i, _len, _ref, + _ref_list, match_ref, provided_keys, build_kwargs; + + _arguments = arguments; _ref_list = self.url_patterns[url_pattern]; + + if (arguments.length == 1 && typeof (arguments[0]) == "object") { + // kwargs mode + provided_keys = new Set (Object.keys (arguments[0])); + + match_ref = function (ref) + { + var _i; + + // Verify that they have the same number of arguments + if (ref[1].length != provided_keys.size) + return false; + + for (_i = 0; + _i < ref[1].length && provided_keys.has (ref[1][_i]); + _i++); + + // If for loop completed, we have all keys + return _i == ref[1].length; + } + + build_kwargs = function (keys) {return _arguments[0];} + + } else { + // args mode + match_ref = function (ref) + { + return ref[1].length == _arguments.length; + } + + build_kwargs = function (keys) { + var kwargs = {}; + + for (var i = 0; i < keys.length; i++) { + kwargs[keys[i]] = _arguments[i]; + } + + return kwargs; + } + } + for (_i = 0; - _i < _ref_list.length && - (_ref = _ref_list[_i], _ref[1].length != arguments.length); + _i < _ref_list.length && !match_ref (_ref_list[_i]); _i++); - url = _ref[0], url_args = _ref[1]; - for (index = _i = 0, _len = url_args.length; _i < _len; index = ++_i) { - url_arg = url_args[index]; - url = url.replace("%(" + url_arg + ")s", arguments[index] || ''); + // can't find a match + if (_i == _ref_list.length) + return null; + + _ref = _ref_list[_i]; + url = _ref[0], url_args = build_kwargs (_ref[1]); + for (url_arg in url_args) { + url = url.replace("%(" + url_arg + ")s", + url_args[url_arg] || ''); } return '{{url_prefix|escapejs}}' + url; }; From fad1be381c5a46596cd2f26c8588a8b5a907f01c Mon Sep 17 00:00:00 2001 From: Chow Loong Jin Date: Thu, 14 May 2015 17:03:50 +0800 Subject: [PATCH 3/4] Add test-case for key-based url reversing --- tests/test_urls.py | 5 ++++- tests/unit_tests.py | 10 +++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/test_urls.py b/tests/test_urls.py index 5c3eeb6..a3dc5dc 100644 --- a/tests/test_urls.py +++ b/tests/test_urls.py @@ -34,7 +34,10 @@ def dummy_view(*args, **kwargs): url(r'^test_duplicate_name/(?P[-\w]+)/$', dummy_view, name='test_duplicate_name'), url(r'^test_duplicate_name/(?P[-\w]+)-(?P[-\w]+)/$', dummy_view, - name='test_duplicate_name')) + name='test_duplicate_name'), + url(r'^test_duplicate_argcount/(?P[-\w]+)?-(?P[-\w]+)?/$', dummy_view, + name='test_duplicate_argcount'), + ) urlpatterns = copy(basic_patterns) diff --git a/tests/unit_tests.py b/tests/unit_tests.py index 83a657e..ede970f 100755 --- a/tests/unit_tests.py +++ b/tests/unit_tests.py @@ -119,13 +119,21 @@ def test_script_prefix(self): with script_prefix('/foobarlala/'): self.assertEqualJSUrlEval('Urls["nestedns:ns1:test_two_url_args"]("arg_one", "arg_two")', '/foobarlala/nestedns/ns1/test_two_url_args/arg_one-arg_two/') - + def test_duplicate_name(self): self.assertEqualJSUrlEval('Urls.test_duplicate_name("arg_one")', '/test_duplicate_name/arg_one/') self.assertEqualJSUrlEval('Urls.test_duplicate_name("arg_one", "arg_two")', '/test_duplicate_name/arg_one-arg_two/') + def test_duplicate_argcount(self): + self.assertEqualJSUrlEval('Urls.test_duplicate_argcount ({arg_one: "arg_one"})', + '/test_duplicate_argcount/arg_one-/') + self.assertEqualJSUrlEval('Urls.test_duplicate_argcount ({arg_two: "arg_two"})', + '/test_duplicate_argcount/-arg_two/') + self.assertEqualJSUrlEval('Urls.test_duplicate_argcount ({arg_one: "arg_one", arg_two: "arg_two"})', + '/test_duplicate_argcount/arg_one-arg_two/') + @override_settings(JS_REVERSE_JS_MINIFY=False) class JSReverseViewTestCaseNotMinified(JSReverseViewTestCaseMinified): From df1ea0afc36e851d57c23197404d6ec39bb925c1 Mon Sep 17 00:00:00 2001 From: Chow Loong Jin Date: Thu, 14 May 2015 18:10:04 +0800 Subject: [PATCH 4/4] Replace usage of Set with a standard dict phantomjs does not appear to support Set yet, which causes the key-based url reversing test cases to fail. --- .../templates/django_js_reverse/urls_js.tpl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/django_js_reverse/templates/django_js_reverse/urls_js.tpl b/django_js_reverse/templates/django_js_reverse/urls_js.tpl index 699fad2..e094649 100644 --- a/django_js_reverse/templates/django_js_reverse/urls_js.tpl +++ b/django_js_reverse/templates/django_js_reverse/urls_js.tpl @@ -16,18 +16,21 @@ if (arguments.length == 1 && typeof (arguments[0]) == "object") { // kwargs mode - provided_keys = new Set (Object.keys (arguments[0])); + var provided_keys_list = Object.keys (arguments[0]); + provided_keys = {}; + for (_i = 0; _i < provided_keys_list.length; _i++) + provided_keys[provided_keys_list[_i]] = 1; match_ref = function (ref) { var _i; // Verify that they have the same number of arguments - if (ref[1].length != provided_keys.size) + if (ref[1].length != provided_keys_list.length) return false; for (_i = 0; - _i < ref[1].length && provided_keys.has (ref[1][_i]); + _i < ref[1].length && ref[1][_i] in provided_keys; _i++); // If for loop completed, we have all keys