Strip some chars from tags

RESOLVED FIXED

Status

Webmaker
MakeAPI
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: cade, Assigned: cade)

Tracking

unspecified
x86
Mac OS X
Bug Flags:
sec-review -

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
For safety, lets remove most chars from tags on saves, updates, and check on searches.
(Assignee)

Comment 1

4 years ago
Created attachment 831908 [details] [review]
https://github.com/mozilla/MakeAPI/pull/170
Attachment #831908 - Flags: review?(pomax)
Comment on attachment 831908 [details] [review]
https://github.com/mozilla/MakeAPI/pull/170

comments in the PR
Attachment #831908 - Flags: review?(pomax) → review-
(Assignee)

Updated

4 years ago
Attachment #831908 - Flags: review- → review?(pomax)
Comment on attachment 831908 [details] [review]
https://github.com/mozilla/MakeAPI/pull/170

winner.
Attachment #831908 - Flags: review?(pomax) → review+

Comment 4

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

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

Comment 5

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

https://github.com/mozilla/MakeAPI/commit/f1250cdf183361bd6b61393ac8f244d2b9e53446
Bug 938424 - escape '-'
I don't mind characters being stripped if they're definitely not needed but the correct fix here is to resolve bug 938109 properly.
Flags: sec-review-
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).
(Assignee)

Comment 8

4 years ago
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.
Flags: needinfo?(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.
Flags: needinfo?(pomax)

Comment 10

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

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

Comment 11

4 years ago
I've decided to leave the filtering in, for safety's sake.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.