- Resolve SEC-27 by escaping course name in advanced settings
- Add escape_json_dumps to simplify escaping json in Mako templates
SEC-27: XSS/JS Error in Advanced Settings with invalid course name
On devops recommendation, now handling the potential for an 'inconsistency
window' via a management command instead of a hacky "re-run the data migration"
bash script.
This includes:
* Ability to specify number of processes to run bok-choy tests in
* A forked nose commit to get the multiprocess plugin's logging to work
* A different plugin (xunitmp) must be used for pulling together xunit results
This works by:
* Starting the various servers that are needed for the acceptance test environment
* Running the tests themselves in multiprocess mode
The only time it should include the suffix is when the handler is
explicilty "xmodule_handler", meaning it's an old-style handler
that routes everything. For example, Capa uses one handler for
all its AJAX requests, and only differentiates actions based on
suffix ("get", "problem_check", etc.).
What prompted this change is that LTIDescriptor defines a handler
"lti_2_0_result_rest_handler" which encodes user-specific
information into the suffix. This is a perfectly valid thing to
do, but it blows out the number of named transactions in our
metrics.
This was originally contributed upstream by Stanford, circa 2013.
We neither use nor support this feature in its current implementation,
and in fact, we may never have used this production. Until recently, we
had additional chat/Jabber code [1] (in the form of a Jabber djangoapp in
LMS); context there suggests this feature may have never been more than
a prototype. The original author is no longer on the team, so I can't directly
confirm this on our end.
Do you use this feature?
Stanford had already abandoned this Jabber-backed chat implementation,
in favor of an IRC backend, by the time I joined the team in early 2014.
[1] dbe52a6b13
Both of them dynamically generate specs which close over the iteration
variable of a for loop. Closures capture *references*, not values, and
so when the variable is mutated on loop iteration its new value is
used when the spec is called. This means that instead of running a
spec with n different values, we run the spec n times with the same
value. This is bad.
Having the messeges embedded into onclick attributes was fragile because
it could break if the translated string contained special characters
such as single quotes.
Rather than trying to escape the special characters inside the onclick handler,
move the translatable strings into separate h-escaped data attributes and interpolate
the values in the (already existing) jQuery click handler rather than in the
inline onclick handler.
This makes the javascript code more consistent as well as fixes the problem
where the popup would break if translation included single quotes.
The read-only protection is more hassle than it's worth when testing locally and on stage. It's a solution to a problem we don't actually have, so I have reverted the changes.
