Fix invalid token.xml template by adding closing slash to Pause tags

[gherceg]

Description

In order for XML to be well formed, all elements must have closing tags. Currently, the token.xml template violates this with the unclosed <Pause> tags. As described under How Has This Been Tested, I came to the conclusion that the invalid token.xml template results in TwiML that Twilio is unable to parse when making phone calls.

This change was introduced in #488 as part of the 1.14.0 release almost two years ago, but I didn't see any open or closed issues related to the phone call functionality being broken which seemed surprising. I briefly looked through Twilio's changelog to see if there were any Twiml related changes that could explain why this might only be an issue with certain versions of Twilio, but nothing jumped out.

I'm admittedly optimistic that this is a straightforward change that is good to make regardless of how broad the impact is.

Motivation and Context

I'm in the process of upgrading a project I work on from v1.13.2 to v1.15.5, and encountered an error with the phone call method. The call is made successfully, but after pressing any key to continue, the response is "We're sorry. An application error has occurred. Good bye". After looking at the Twilio log for this call, I noticed that this tag is never closed and figured that might be the cause of the error, despite Twilio confirming that the POST request to twilio/inbound/two_factor/<token> was successful.

How Has This Been Tested?

I pasted the following simplified xml into both Twilio's TwiML Bin and a regular XML validator as a sanity check, and both confirmed that the unclosed <Pause> tag was invalid, but updating to <Pause/> was valid.

Invalid

<?xml version="1.0" encoding="UTF-8" ?>
<Response>
  <Pause>
</Response>

Valid

<?xml version="1.0" encoding="UTF-8" ?>
<Response>
  <Pause/>
</Response>

I also updated the automated test that verifies the generated XML is as expected.

Update: I was also able to test this on the project I'm upgrading and confirmed that this change resolves the error encountered with the phone call method.

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[claudep]

Thanks a lot for this important fix! Could you still explore any capability in Python XML libs to validate if the content is XML compliant (for some regression test)?

[claudep]

Maybe something like:

from xml.sax.handler import ContentHandler
from xml.sax import make_parser
parser = make_parser()
parser.setContentHandler(ContentHandler())
parser.parseString(<xmlcontent>)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.84%. Comparing base (1928748) to head (f071908).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #708   +/-   ##
=======================================
  Coverage   97.84%   97.84%           
=======================================
  Files          78       78           
  Lines        3384     3389    +5     
  Branches      384      384           
=======================================
+ Hits         3311     3316    +5     
  Misses         42       42           
  Partials       31       31           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[gherceg]

Good point. It looks like we can do that with xml.etree.ElementTree.fromstring so I've added that in f071908. As you'll see, I typically just add a comment to reflect that it'll raise an exception if it fails, but if you have a preference for how to test that specific behavior I'm happy to update that.

I confirmed that the test will fail if the unclosed tag is included in either press_a_key.xml or token.xml, and here is an example of that output for reference.

ERROR: test_call_app (tests.test_gateways.TwilioGatewayTest.test_call_app)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/gherceg/dimagi/code/django-two-factor-auth/tests/test_gateways.py", line 36, in test_call_app
    ElementTree.fromstring(decoded_response)
  File "/home/gherceg/.pyenv/versions/3.11.6/lib/python3.11/xml/etree/ElementTree.py", line 1338, in XML
    parser.feed(text)
xml.etree.ElementTree.ParseError: mismatched tag: line 32, column 2

----------------------------------------------------------------------
Ran 1 test in 0.008s

FAILED (errors=1)

Or if you prefer it to be a separate test altogether that just checks the xml templates directly, I could do that instead.

[claudep]

Thanks, that's what I had in mind!

[claudep]

🥇

[gherceg]

Thanks for the quick review! Is there a timeline for the next release? Just useful for planning purposes so no worries if there isn't one.

[claudep]

No timeline for now. I guess you'll have to use the git master in the meantime.