Page MenuHomePhabricator

requestctl should handle (or reject) quotation marks in the resp_status
Open, Needs TriagePublic

Description

Today @RKemper tried to write an action with

resp_code: 403
resp_status: A status message that includes double quotes, "like this".

That doesn't cause any problems in YAML parsing: it ends up as a string containing two " characters with no trouble. But the generated VCL is syntactically invalid:

return (synth(403, "A status message that includes double quotes, "like this"."));

We should have noticed it when requestctl commit presented the diff, but during the outage we missed it. All the cache-text hosts started alerting with

18:14:24 <icinga-wm> PROBLEM - Confd vcl based reload on cp4039 is CRITICAL: reload-vcl failed to run since 0h, 3 minutes. https://wikitech.wikimedia.org/wiki/Varnish

So, what's good: Varnish didn't crash on the invalid VCL, which would have instantly been a global outage. And the alert spam brought the problem to our attention, so we could promptly roll back the change and try again.

But what could be better: requestctl should have emitted a VCL long string, like

return (synth(403, {"A status message that includes double quotes, "like this"."}));

or at least printed an error.