Merge lp:~danilo/launchpad/bug-740640 into lp:launchpad

Proposed by Данило Шеган
Status: Merged
Approved by: Данило Шеган
Approved revision: no longer in the source branch.
Merged at revision: 12691
Proposed branch: lp:~danilo/launchpad/bug-740640
Merge into: lp:launchpad
Prerequisite: lp:~danilo/launchpad/accordionoverlay
Diff against target: 395 lines (+267/-5)
5 files modified
lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py (+8/-0)
lib/lp/bugs/model/bugsubscriptionfilter.py (+17/-1)
lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py (+27/-0)
lib/lp/registry/javascript/structural-subscription.js (+119/-4)
lib/lp/registry/javascript/tests/test_structural_subscription.js (+96/-0)
To merge this branch: bzr merge lp:~danilo/launchpad/bug-740640
Reviewer Review Type Date Requested Status
Henning Eggers (community) ui,code Approve
Registry Administrators ui Pending
Review via email: mp+55123@code.launchpad.net

Commit message

[r=henninge][ui=henninge][bug=740640] Protect against XSS in structural subscription overlay and add nicer client-side validation.

Description of the change

= Bug 740640 =

At the moment, structural subscription overlay allows XSS attacks.

We should stop them at the server-side and provide nice JS-based validation to inform the users of the conditions.

Since we want similar approach for client-side tags validation, this branch implements that as well.

== Pre-implementation notes ==

I've discussed approach to solving this with Gary: allowing everything in the subscription name, while probably the best approach, would complicate implementation in that existing infrastructure just populates the data from the server-side objects directly. Thus, we decided on black-listing potential XSS-attack characters for subscription name.

== Implementation details ==

For server-side, we implement "description" property through setter and getter.

Server-side changes:
 - bugs/model/bugsubscriptionfilter.py
 - bugs/browser/tests/test_bugsubscriptionfilter.py
 - bugs/model/tests/test_bugsubscriptionfilter.py

Client-side changes:
 - All the rest

== Tests ==

  bin/test -cvvt BugSubscriptionFilter.*description
  lib/lp/registry/javascript/tests/test_structural_subscription.html

== Demo and Q/A ==

See also https://devpad.canonical.com/~danilo/screencasts/inline-validation-error.ogv

  1. Go to https://launchpad.dev/+feature-rules and add "advanced-structural-subscriptions.enabled default 10 on"
  2. Go to http://launchpad.dev/firefox and click on "Subscribe to bug mail" (on the right) and try entering "<", ">", "&" or "\"" into the subscription name: check gets triggered on the 'change' event, which means when you focus on the next element
  3. Similarly, expand "more options" and try adding a few invalid tags (valid tags are "[a-z0-9][a-z0-9+.-]*", i.e. all lowercase or digits with +, ., -)
  4. Try saving with the errors still present

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/templates/product-index.pt
  lib/lp/registry/help/structural-subscription-name.html
  lib/lp/registry/help/structural-subscription-tags.html
  lib/lp/bugs/browser/structuralsubscription.py
  lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py
  lib/lp/registry/javascript/tests/test_structural_subscription.html
  lib/lp/app/javascript/lp.js
  lib/lp/registry/browser/product.py
  lib/lp/bugs/browser/tests/test_expose.py
  lib/lp/bugs/model/bugsubscriptionfilter.py
  lib/lp/registry/templates/product-portlet-license-missing.pt
  lib/lp/bugs/templates/bug-subscription-list.pt
  lib/lp/bugs/templates/bugtarget-subscription-list.pt
  lib/lp/services/inlinehelp/javascript/inlinehelp.js
  lib/lp/registry/javascript/tests/test_structural_subscription.js
  lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py
  lib/lp/registry/javascript/structural-subscription.js
  buildout-templates/bin/combine-css.in
  lib/lp/app/javascript/tests/test_accordionoverlay.html

./lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py
      82: E301 expected 1 blank line, found 0

To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :
Download full text (10.3 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Danilo,
thanks for this important fix, I am sorry it took a bit longer to review it.
Also thanks for the discussion on IRC, I really enjoyed getting behind what
this code does (and I almost fully succeeded ... ;-)

With the issues solved as discussed, this is good to land code-wise. Please
consider my other two suggestions below.

 review approve code

Cheers,
Henning

Am 28.03.2011 13:22, schrieb Данило Шеган:
> === modified file 'lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py'
> --- lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-02-18 18:16:34 +0000
> +++ lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-03-28 11:22:31 +0000
> @@ -183,6 +183,15 @@
> self.assertEqual(
> u"It's late.", self.subscription_filter.description)
>
> + def test_modify_description_xss_safeguard(self):
> + # The description can be modified.
> +
> + # Modify, save, and start a new transaction.

Oops, something wrong with the comments here ...

> + self.ws_subscription_filter.description = u"&gt; <>"
> + error = self.assertRaises(
> + BadRequest, self.ws_subscription_filter.lp_save)
> + self.assertEqual(400, error.response.status)
> +
> def test_modify_statuses(self):
> # The statuses field can be modified.
> self.assertEqual(set(), self.subscription_filter.statuses)
>
> === modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
> --- lib/lp/bugs/model/bugsubscriptionfilter.py 2011-03-22 14:53:26 +0000
> +++ lib/lp/bugs/model/bugsubscriptionfilter.py 2011-03-28 11:22:31 +0000
> @@ -22,6 +22,7 @@
> from canonical.database.sqlbase import sqlvalues
> from canonical.launchpad import searchbuilder
> from canonical.launchpad.interfaces.lpstorm import IStore
> +from lazr.restful.error import expose
> from lp.bugs.enum import BugNotificationLevel
> from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
> from lp.bugs.interfaces.bugtask import (
> @@ -62,7 +63,20 @@
>
> other_parameters = Unicode()
>
> - description = Unicode()
> + _description = Unicode('description')
> +
> + def _get_description(self):
> + return self._description
> +
> + def _set_description(self, description):
> + if ('<' in description or '>' in description or
> + '"' in description or '&' in description):

I don't like multi-line conditions. Can you please use an intermediate
variable? Ore would a regular expression be too expensive here?
re.find('[<>"&]', description), I think.

> + raise expose(ValueError(
> + 'BugSubscriptionFilter description cannot contain '
> + 'any of <, >, " or &.'))
> + self._description = description
> +
> + description = property(_get_description, _set_description)
>
> def _get_statuses(self):
> """Return a frozenset of statuses to filter on."""
>
> === modified file 'lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py'
> --- lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py 2011-03-21 18:20:09 +0000
> +++ lib/lp/bugs/model/tests/test_bugsubscriptionf...

review: Approve (code)
Revision history for this message
Henning Eggers (henninge) wrote :

The only UI issue I see is the question if the error text for the tags is correct. Should it not mention that tags can only begin with an alphanumeric character? Otherwise we might have users wondering what they are doing wrong by entering ".foo".

review: Approve (ui,code)
Revision history for this message
Данило Шеган (danilo) wrote :
Download full text (6.5 KiB)

У пон, 28. 03 2011. у 15:24 +0000, Henning Eggers пише:
> Review: Approve code
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi Danilo,
> thanks for this important fix, I am sorry it took a bit longer to review it.
> Also thanks for the discussion on IRC, I really enjoyed getting behind what
> this code does (and I almost fully succeeded ... ;-)
>
> With the issues solved as discussed, this is good to land code-wise. Please
> consider my other two suggestions below.
>
> review approve code
>
> Cheers,
> Henning
>
>
> Am 28.03.2011 13:22, schrieb Данило Шеган:
> > === modified file 'lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py'
> > --- lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-02-18 18:16:34 +0000
> > +++ lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-03-28 11:22:31 +0000
> > @@ -183,6 +183,15 @@
> > self.assertEqual(
> > u"It's late.", self.subscription_filter.description)
> >
> > + def test_modify_description_xss_safeguard(self):
> > + # The description can be modified.
> > +
> > + # Modify, save, and start a new transaction.
>
> Oops, something wrong with the comments here ...

Fixed :)

> > === modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
> > --- lib/lp/bugs/model/bugsubscriptionfilter.py 2011-03-22 14:53:26 +0000
> > +++ lib/lp/bugs/model/bugsubscriptionfilter.py 2011-03-28 11:22:31 +0000

> > @@ -62,7 +63,20 @@
> >
> > other_parameters = Unicode()
> >
> > - description = Unicode()
> > + _description = Unicode('description')
> > +
> > + def _get_description(self):
> > + return self._description
> > +
> > + def _set_description(self, description):
> > + if ('<' in description or '>' in description or
> > + '"' in description or '&' in description):
>
> I don't like multi-line conditions. Can you please use an intermediate
> variable? Ore would a regular expression be too expensive here?
> re.find('[<>"&]', description), I think.

I did consider using the re module (but for a very short while). It's
not about multi-line for me though, but mostly about performance: this
would do four passes over the string, and I suspect re would be even
faster for long strings. However, with short strings (like descriptions
will be), burden of loading the re module and compiling the expression
is probably more than that.

I'm happy to put it into an intermediate variable though, so I did that.

> > === modified file 'lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py'
> > --- lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py 2011-03-21 18:20:09 +0000
> > +++ lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py 2011-03-28 11:22:31 +0000
> > @@ -70,6 +70,21 @@
> > self.assertEqual(u"foo", bug_subscription_filter.other_parameters)
> > self.assertEqual(u"bar", bug_subscription_filter.description)
> >
> > + def test_description(self):
> > + """Test the description property."""
> > + bug_subscription_filter = BugSubscriptionFilter()
> > + bug_subscription_filter.description = u"foo"
> > + self.assertEqual(u"foo", bug_subscription_filter.descrip...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py'
2--- lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-02-18 18:16:34 +0000
3+++ lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-03-29 13:11:26 +0000
4@@ -183,6 +183,14 @@
5 self.assertEqual(
6 u"It's late.", self.subscription_filter.description)
7
8+ def test_modify_description_xss_safeguard(self):
9+ # The description can be modified but raises an
10+ # exception if you try potential XSS attack characters.
11+ self.ws_subscription_filter.description = u"&gt; <>"
12+ error = self.assertRaises(
13+ BadRequest, self.ws_subscription_filter.lp_save)
14+ self.assertEqual(400, error.response.status)
15+
16 def test_modify_statuses(self):
17 # The statuses field can be modified.
18 self.assertEqual(set(), self.subscription_filter.statuses)
19
20=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
21--- lib/lp/bugs/model/bugsubscriptionfilter.py 2011-03-23 15:55:44 +0000
22+++ lib/lp/bugs/model/bugsubscriptionfilter.py 2011-03-29 13:11:26 +0000
23@@ -22,6 +22,7 @@
24 from canonical.database.sqlbase import sqlvalues
25 from canonical.launchpad import searchbuilder
26 from canonical.launchpad.interfaces.lpstorm import IStore
27+from lazr.restful.error import expose
28 from lp.bugs.enum import BugNotificationLevel
29 from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
30 from lp.bugs.interfaces.bugtask import (
31@@ -62,7 +63,22 @@
32
33 other_parameters = Unicode()
34
35- description = Unicode()
36+ _description = Unicode('description')
37+
38+ def _get_description(self):
39+ return self._description
40+
41+ def _set_description(self, description):
42+ has_html_chars = (
43+ '<' in description or '>' in description or
44+ '"' in description or '&' in description)
45+ if has_html_chars:
46+ raise expose(ValueError(
47+ 'BugSubscriptionFilter description cannot contain '
48+ 'any of <, >, " or &.'))
49+ self._description = description
50+
51+ description = property(_get_description, _set_description)
52
53 def _get_statuses(self):
54 """Return a frozenset of statuses to filter on."""
55
56=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py'
57--- lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py 2011-03-21 18:23:31 +0000
58+++ lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py 2011-03-29 13:11:26 +0000
59@@ -70,6 +70,33 @@
60 self.assertEqual(u"foo", bug_subscription_filter.other_parameters)
61 self.assertEqual(u"bar", bug_subscription_filter.description)
62
63+ def test_description(self):
64+ """Test the description property."""
65+ bug_subscription_filter = BugSubscriptionFilter()
66+ bug_subscription_filter.description = u"foo"
67+ self.assertEqual(u"foo", bug_subscription_filter.description)
68+
69+ def test_description_xss_safeguard_tags(self):
70+ """Test that description property disallows a HTML tags."""
71+ bug_subscription_filter = BugSubscriptionFilter()
72+ self.assertRaises(ValueError,
73+ setattr, bug_subscription_filter, 'description',
74+ u'<script>')
75+
76+ def test_description_xss_safeguard_ampersand(self):
77+ """description property disallows a few HTML characters."""
78+ bug_subscription_filter = BugSubscriptionFilter()
79+ self.assertRaises(ValueError,
80+ setattr, bug_subscription_filter, 'description',
81+ u'test & blow up')
82+
83+ def test_description_xss_safeguard_quote(self):
84+ """description property disallows a few HTML characters."""
85+ bug_subscription_filter = BugSubscriptionFilter()
86+ self.assertRaises(ValueError,
87+ setattr, bug_subscription_filter, 'description',
88+ u'test "double" quotes')
89+
90 def test_defaults(self):
91 """Test the default values of `BugSubscriptionFilter` objects."""
92 # Create.
93
94=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
95--- lib/lp/registry/javascript/structural-subscription.js 2011-03-28 19:24:42 +0000
96+++ lib/lp/registry/javascript/structural-subscription.js 2011-03-29 13:11:26 +0000
97@@ -208,6 +208,9 @@
98
99 function save_subscription(form_data) {
100 var who;
101+ var has_errors = check_for_errors_in_overlay(add_subscription_overlay);
102+ if (has_errors)
103+ return false;
104 if (form_data.recipient[0] === 'user') {
105 who = LP.links.me;
106 } else {
107@@ -218,10 +221,36 @@
108 }
109 namespace.save_subscription = save_subscription;
110
111+function check_for_errors_in_overlay(overlay) {
112+ var has_errors = false;
113+ var errors = new Array();
114+ for (var field in overlay.field_errors) {
115+ if (overlay.field_errors[field]) {
116+ has_errors = true;
117+ errors.push(field);
118+ }
119+ };
120+ if (has_errors) {
121+ var error_text = errors.pop()
122+ if (errors.length > 0) {
123+ error_text = errors.join(', ') + ' and ' + error_text;
124+ }
125+
126+ overlay.showError(
127+ 'Value for ' + error_text + ' is invalid.');
128+ return true;
129+ } else {
130+ return false;
131+ }
132+};
133+
134 /**
135 * Handle the activation of the edit subscription link.
136 */
137 function edit_subscription_handler(context, form_data) {
138+ var has_errors = check_for_errors_in_overlay(add_subscription_overlay);
139+ if (has_errors)
140+ return false;
141 var on = {success: function (new_data) {
142 var filter = new_data.getAttrs();
143 var description_node = Y.one(
144@@ -237,6 +266,18 @@
145 patch_bug_filter(context.filter_info.filter, form_data, on);
146 }
147
148+function setup_overlay_validators(overlay, overlay_id) {
149+ overlay.field_validators = {
150+ name: get_error_for_subscription_name,
151+ tags: get_error_for_tags_list
152+ };
153+ overlay.field_errors = {};
154+ for (var field_name in overlay.field_validators) {
155+ add_input_validator(overlay, overlay_id, field_name,
156+ overlay.field_validators[field_name]);
157+ };
158+};
159+
160 /**
161 * Populate the overlay element with the contents of the add/edit form.
162 */
163@@ -252,9 +293,15 @@
164 visible: false,
165 form_submit_button: submit_button,
166 form_cancel_button: Y.Node.create(cancel_button_html),
167- form_submit_callback: submit_callback
168+ form_submit_callback: function(formdata) {
169+ // Do not clean up if saving was not successful.
170+ var save_succeeded = submit_callback(formdata);
171+ if (save_succeeded !== false)
172+ clean_up();
173+ }
174 });
175 add_subscription_overlay.render(content_box_id);
176+ setup_overlay_validators(add_subscription_overlay, overlay_id);
177 // Prevent cruft from hanging around upon closing.
178 function clean_up(e) {
179 var filter_wrapper = Y.one('#' + FILTER_WRAPPER);
180@@ -263,8 +310,6 @@
181 }
182 add_subscription_overlay.get('form_cancel_button').on(
183 'click', clean_up);
184- add_subscription_overlay.get('form_submit_button').on(
185- 'click', clean_up);
186 add_subscription_overlay.on('cancel', clean_up);
187 }
188
189@@ -396,6 +441,11 @@
190 };
191 }
192
193+/* We want to call 'resize' directly on tags container
194+ * when we add a validation failed error message.
195+ */
196+var tags_container;
197+
198 /**
199 * Create the accordion.
200 *
201@@ -426,6 +476,8 @@
202 contentHeight: {method: "auto"}
203 } );
204
205+ tags_container = tags_ai;
206+
207 tags_ai.set("bodyContent",
208 '<div>\n' +
209 '<div>\n' +
210@@ -774,6 +826,70 @@
211 }, window);
212 };
213
214+/*
215+ * Set up a validator function for a form input field.
216+ * @method add_input_validator
217+ * @param {Object} overlay Overlay object.
218+ * @param {String} overlay_id Element ID of the containing overlay.
219+ * @param {String} field_name Form <input> 'name' to set up a validator for.
220+ * @param {String} validator Function which returns 'null' if there is
221+ no error in the field value, and an error message otherwise.
222+ */
223+function add_input_validator(overlay, overlay_id, field_name, validator) {
224+ var input = Y.one(overlay_id + ' input[name="'+field_name+'"]');
225+ var field_container = input.get('parentNode');
226+ var error_container = Y.Node.create('<div class="inline-warning"></div>');
227+ field_container.appendChild(error_container);
228+
229+ input.on('change', function(e) {
230+ var error_text = validator(input.get('value'));
231+ if (error_text !== null) {
232+ Y.lazr.anim.red_flash({node: input}).run();
233+ error_container.setContent(error_text);
234+ overlay.field_errors[field_name] = true;
235+ // Accordion sets fixed height for the accordion item,
236+ // so we have to resize the tags container.
237+ if (field_name == 'tags')
238+ tags_container.resize();
239+ // Firefox prohibits focus from inside the 'focus lost' event
240+ // handler (probably to stop loops), so we need to run
241+ // it from a different context (which we do with setTimeout).
242+ setTimeout(function() { input.focus(); input.select(); }, 1);
243+ } else {
244+ error_container.setContent('');
245+ overlay.field_errors[field_name] = false;
246+ }
247+ });
248+};
249+
250+function get_error_for_subscription_name(value) {
251+ if (value.search(/[\<\>\"\&]/) == -1) {
252+ return null;
253+ } else {
254+ return ('Subscription name cannot contain any of the following ' +
255+ 'characters: &lt; &gt; &quot; &amp;');
256+ }
257+};
258+// Export for testing
259+namespace._get_error_for_subscription_name = get_error_for_subscription_name;
260+
261+function get_error_for_tags_list(value) {
262+ // See database/schema/trusted.sql valid_name() function
263+ // which is used to validate a single tag.
264+ // As an extension, we also allow "-" (hyphen) in front of
265+ // any tag to indicate exclusion of a tag, and we accept
266+ // a space-separated list.
267+ if (value.match(/^(\-?[a-z0-9][a-z0-9\+\.\-]*[ ]*)*$/) !== null) {
268+ return null;
269+ } else {
270+ return ('Tags can only contain lowercase ASCII letters, ' +
271+ 'digits 0-9 and symbols "+", "-" or ".", and they ' +
272+ 'must start with a lowercase letter or a digit.');
273+ }
274+};
275+// Export for testing
276+namespace._get_error_for_tags_list = get_error_for_tags_list;
277+
278 function get_input_by_value(node, value) {
279 // XXX broken: this should also care about input name because some values
280 // repeat in other areas of the form
281@@ -1180,7 +1296,6 @@
282 // initialized except for the ones we added, so setupHelpTrigger
283 // is idempotent. Notice that this is old MochiKit code.
284 forEach(findHelpLinks(), setupHelpTrigger);
285-
286 }; // setup
287
288 }, '0.1', {requires: [
289
290=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
291--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-03-28 19:24:42 +0000
292+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-03-29 13:11:26 +0000
293@@ -235,6 +235,102 @@
294 suite.add(test_case);
295
296 test_case = new Y.Test.Case({
297+ name: 'Structural Subscription validation tests',
298+
299+ _should: {
300+ error: {
301+ }
302+ },
303+
304+ setUp: function() {
305+ // Monkeypatch LP to avoid network traffic and to allow
306+ // insertion of test data.
307+ window.LP = {
308+ links: {},
309+ cache: {}
310+ };
311+
312+ },
313+
314+ test_get_error_for_subscription_name_valid: function() {
315+ // Valid name makes get_error_for_subscription_name return null.
316+ var name = 'Test + name!';
317+ Assert.isNull(module._get_error_for_subscription_name(name));
318+ },
319+
320+ assertHasErrorInSubscriptionName: function(subscription_name) {
321+ var error_text = module._get_error_for_subscription_name(
322+ subscription_name);
323+ Assert.isNotNull(error_text);
324+ Assert.areEqual(
325+ 'Subscription name cannot contain any of the following ' +
326+ 'characters: &lt; &gt; &quot; &amp;',
327+ error_text);
328+ },
329+
330+ test_get_error_for_subscription_name_left_angle_bracket: function() {
331+ // For invalid names get_error_for_subscription_name returns
332+ // an error message instead.
333+ this.assertHasErrorInSubscriptionName('<Test');
334+ },
335+
336+ test_get_error_for_subscription_name_right_angle_bracket: function() {
337+ // For invalid names get_error_for_subscription_name returns
338+ // an error message instead.
339+ this.assertHasErrorInSubscriptionName('Test>');
340+ },
341+
342+ test_get_error_for_subscription_name_ampersand: function() {
343+ // For invalid names get_error_for_subscription_name returns
344+ // an error message instead.
345+ this.assertHasErrorInSubscriptionName('Test & fail');
346+ },
347+
348+ test_get_error_for_subscription_name_quote: function() {
349+ // For invalid names get_error_for_subscription_name returns
350+ // an error message instead.
351+ this.assertHasErrorInSubscriptionName('Test "failure"');
352+ },
353+
354+ test_get_error_for_tags_list_valid: function() {
355+ // Valid tags list is a space-separated list of tags
356+ // consisting of all lowercase and digits and potentially
357+ // '+', '-', '.' in non-initial characters.
358+ var tags = 'tag1 tag+2 tag.3 tag-4 5tag';
359+ Assert.isNull(module._get_error_for_tags_list(tags));
360+ },
361+
362+ assertHasErrorInTagsList: function(tags) {
363+ var error_text = module._get_error_for_tags_list(tags);
364+ Assert.isNotNull(error_text);
365+ Assert.areEqual(
366+ 'Tags can only contain lowercase ASCII letters, ' +
367+ 'digits 0-9 and symbols "+", "-" or ".", and they ' +
368+ 'must start with a lowercase letter or a digit.',
369+ error_text);
370+ },
371+
372+
373+ test_get_error_for_tags_list_uppercase: function() {
374+ // Uppercase is not allowed in tags.
375+ this.assertHasErrorInTagsList('Tag');
376+ },
377+
378+ test_get_error_for_tags_list_invalid_characters: function() {
379+ // Anything other than lowercase, digits or '+', '-' and '.'
380+ // is invalid in tags.
381+ this.assertHasErrorInTagsList('tag#!');
382+ },
383+
384+ test_get_error_for_tags_list_special_characters: function() {
385+ // Even if '+', '-' or '.' are allowed in tags,
386+ // they must not be at the beginning of a tag.
387+ this.assertHasErrorInTagsList('tag1 +tag2 -tag3 .tag4');
388+ },
389+ });
390+ suite.add(test_case);
391+
392+ test_case = new Y.Test.Case({
393 name: 'Structural Subscription interaction tests',
394
395 _should: {