Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Twisted WebSocket test build failure #452

Open
nigelmegitt opened this issue Jun 23, 2017 · 5 comments
Open

Twisted WebSocket test build failure #452

nigelmegitt opened this issue Jun 23, 2017 · 5 comments

Comments

@nigelmegitt
Copy link
Collaborator

Unexpected build failure with no change in our code:

================================================================================== FAILURES ==================================================================================
______________________________________________________ TestProdServerToConsClientProtocols.test_url_encoded_components _______________________________________________________

self = <test_twisted_websocket.TestProdServerToConsClientProtocols testMethod=test_url_encoded_components>

    def test_url_encoded_components(self):
        # This test is about getting percent encoded characters work in sequenceId or hostname
        sequence_id = u'sequence/ünicödé?/Name'
        self._create_server(url='ws://localhost:9006', producer=self.prod)
        self._create_client(
            url='ws://localhost:9006/sequence%2F%C3%BCnic%C3%B6d%C3%A9%3F%2FName/subscribe',
            consumer=self.cons
        )
    
        self.cproto.consumer = self.cons
    
>       self._connect()

ebu_tt_live/twisted/test/test_twisted_websocket.py:214: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
ebu_tt_live/twisted/test/test_twisted_websocket.py:47: in _connect
    self.assertEquals(self.sproto.state, self.sproto.STATE_OPEN)
venv/lib/python2.7/site-packages/twisted/trial/_synctest.py:432: in assertEqual
    super(_Assertions, self).assertEqual(first, second, msg)
E   FailTest: 0 != 3
-------------------------------------------------------------------------------- Captured log --------------------------------------------------------------------------------
websocket.py               227 ERROR    one or more reserved delimiters /?# present in path segment: u'sequence/\xfcnic\xf6d\xe9?/Name'
websocket.py               352 ERROR    one or more reserved delimiters /?# present in path segment: u'sequence/\xfcnic\xf6d\xe9?/Name'

Presumably this has been induced by a change to a dependency.

@nigelmegitt
Copy link
Collaborator Author

I've narrowed down the cause of this, after much painful digging. See python-hyper/hyperlink@d26814c#commitcomment-22765690

My explanation:

  • Twisted uses hyperlink.URL
  • hyperlink.URL.from_text() now calls _textcheck() on the path segment looking for the characters /?# which in our test at test_twisted_websocket.test_url_encoded_components() fails.
  • So we get a value error returned on the basis that one or more reserved delimiters are present in the path segment. This of course is exactly what our test is checking works correctly.

Having read RFC 3986 and the Wikipedia Percent-encoding article, I believe this is a bug in hyperlink.

I'd like someone (maybe @tomrosier or @kozmaz87 ?) to check that I'm indeed correct in believing that reserved characters can be included in path segments after percent encoding, and then we can raise the issue on hyperlink. Otherwise we'll have to rely on an earlier version of hyperlink, annoyingly.

@nigelmegitt
Copy link
Collaborator Author

Addition to the above: python-hyper/hyperlink#16 is an example where this has already been raised as an issue.

@nigelmegitt
Copy link
Collaborator Author

Pull request #453 passes local tests, by requiring hyperlink < 17.2.0.

@nigelmegitt
Copy link
Collaborator Author

Looks like this issue might close itself when the hyperlink project makes their next release - they expect this bug to be fixed by then.

@nigelmegitt
Copy link
Collaborator Author

Just tested removing the hyperlink version requirement, got:

Installing collected packages: hyperlink, ebu-tt-live
  Found existing installation: hyperlink 17.1.1
    Uninstalling hyperlink-17.1.1:
      Successfully uninstalled hyperlink-17.1.1
  Found existing installation: ebu-tt-live 2.1
    Uninstalling ebu-tt-live-2.1:
      Successfully uninstalled ebu-tt-live-2.1
  Running setup.py develop for ebu-tt-live
Successfully installed ebu-tt-live hyperlink-17.3.1

but then:

=================================================================================== FAILURES ===================================================================================
_______________________________________________________ TestProdServerToConsClientProtocols.test_url_encoded_components ________________________________________________________

self = <test_twisted_websocket.TestProdServerToConsClientProtocols testMethod=test_url_encoded_components>

    def test_url_encoded_components(self):
        # This test is about getting percent encoded characters work in sequenceId or hostname
        sequence_id = u'sequence/ünicödé?/Name'
        self._create_server(url='ws://localhost:9006', producer=self.prod)
        self._create_client(
            url='ws://localhost:9006/sequence%2F%C3%BCnic%C3%B6d%C3%A9%3F%2FName/subscribe',
            consumer=self.cons
        )
    
        self.cproto.consumer = self.cons
    
        self._connect()
    
>       self.assertEquals(sequence_id, self.cproto._sequence_identifier)

ebu_tt_live/twisted/test/test_twisted_websocket.py:216: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
venv/lib/python2.7/site-packages/twisted/trial/_synctest.py:432: in assertEqual
    super(_Assertions, self).assertEqual(first, second, msg)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <test_twisted_websocket.TestProdServerToConsClientProtocols testMethod=test_url_encoded_components>
msg = "u'sequence/\xfcnic\xf6d\xe9?/Name' != u'sequence%2F\xfcnic\xf6d\xe9%3F%2FName'
- sequence/ünicödé?/Name
?         ^       ^^
+ sequence%2Fünicödé%3F%2FName
?         ^^^       ^^^^^^
"

    def fail(self, msg=None):
        """
            Absolutely fail the test.  Do not pass go, do not collect $200.
    
            @param msg: the message that will be displayed as the reason for the
            failure
            """
>       raise self.failureException(msg)
E       FailTest: u'sequence/\xfcnic\xf6d\xe9?/Name' != u'sequence%2F\xfcnic\xf6d\xe9%3F%2FName'
E       - sequence/ünicödé?/Name
E       ?         ^       ^^
E       + sequence%2Fünicödé%3F%2FName
E       ?         ^^^       ^^^^^^

venv/lib/python2.7/site-packages/twisted/trial/_synctest.py:375: FailTest

So we still need this fix in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant