938424 - Strip some chars from tags

Status: RESOLVED FIXED

(Webmaker Graveyard :: MakeAPI, defect)

Description

[Chris DeCairos (:cade)]

For safety, lets remove most chars from tags on saves, updates, and check on searches.

Comment 1

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/MakeAPI/pull/170

Comment 2

[Pomax]

Comment on attachment 831908 [details] [review]
https://github.com/mozilla/MakeAPI/pull/170

comments in the PR

Comment 3

[Pomax]

Comment on attachment 831908 [details] [review]
https://github.com/mozilla/MakeAPI/pull/170

winner.

Comment 4

[[github robot]]

Commit pushed to master at https://github.com/mozilla/MakeAPI

https://github.com/mozilla/MakeAPI/commit/2bba43fb1c7453f70a421be76eeafa2b88c27ef7
Bug 938424 - allow . and /

Comment 5

[[github robot]]

Commit pushed to master at https://github.com/mozilla/MakeAPI

https://github.com/mozilla/MakeAPI/commit/f1250cdf183361bd6b61393ac8f244d2b9e53446
Bug 938424 - escape '-'

Comment 6

[Mark Goodwin [:mgoodwin]]

I don't mind characters being stripped if they're definitely not needed but the correct fix here is to resolve bug 938109 properly.

Comment 7

[Pomax]

agreed, although it won't be encoding the data correctly when serving it back up "instead" of character filtering, but on top of (the makeapi has rules about what counts as valid tag data, and part of the problem here was that these rules were not being enforced).

Comment 8

[Chris DeCairos (:cade)]

With the fix for bug 938109 in place we should consider revisiting this bug before closing it. With that other patch landed, we don't have to filter out certain chars any longer.

Comment 9

[Pomax]

we can remove the specific-character-removal if the new fix properly escapes at the right times. If we remove it, potential badness will still exist at the consumer the moment they run a decodeURIComponent on the makeapi's data. I would still be strongly in favour of also writing a tiny library that lets consumers tell it "safely populate this element with that data" like makeapi.populate($('tags'), "tags") or something. That way we're not passing the buck.

Comment 10

[[github robot]]

Commit pushed to master at https://github.com/mozilla/MakeAPI

https://github.com/mozilla/MakeAPI/commit/f1250cdf183361bd6b61393ac8f244d2b9e53446
Bug 938424 - escape '-'

Comment 11

[Chris DeCairos (:cade)]

I've decided to leave the filtering in, for safety's sake.