| cmelbye | |
| Jul 27 2015, 6:09 PM |
| F696568: Screen Shot 2015-08-08 at 9.24.26 PM.png | |
| Aug 9 2015, 4:25 AM |
| F669858: string.gif | |
| Jul 27 2015, 9:27 PM |
Hello! I think I've found an issue in the Instance Configuration page of Phacility Admin. I tried to set the feed.http_hooks setting to the following:
["https://ever-phab-slack.herokuapp.com"]
However, when I pressed save, it appeared like this:
["https:\/\/ever-phab-slack.herokuapp.com"]
Luckily, the correct (non-escaped) value got sent to my Phabricator instance, and the HTTP hook is working properly. Thus, it appears to be merely a cosmetic issue in Phacility Admin.
This is a valid, compliant JSON-encoding of the string, which allows / to be escaped -- here's the grammar diagram from json.org:
The reason some encoders (including the builtin PHP encoder, which we use) escape this character is to make sure the string </script> appears as <\/script> in output. This defuses some browser vulnerabilities where an attacker could enter, say, a comment like </script><script>..., get that JSON encoded and echoed into the page, and then trick browsers into interpreting the page source as an abruptly-terminated script tag with a syntax error followed by an attacker-controlled script tag with nearly-arbitrary contents.
This encoding isn't desirable when encoding for human users, and we should probably use alternate encoding rules which prevent backslashes from being escaped in these contexts, but the encoding is proper and valid.
The specific UI (Phacility → Instances → (Choose an Instance) → Adjust Configuration) in question doesn't actually use PhutilJSON to format the output right now, so it's still rendering the less-preferred form:
I need to swap that over at some point.
T10358 discusses a case where we use (valid, spec-compliant) JSON for presentation, but could use an alternate form which is more human readable (specifically, when displaying unicode characters).