autobahn-testsuite: Commit f0c83a3 breaks the 12.4 tests.
https://github.com/crossbario/autobahn-testsuite/commit/f0c83a3d6ad157417b7d56091796d8c8798be9c0 makes some changes to the file testdata/data1.html, which is used in the 12.4 block of tests for compression. I don’t yet understand why, but these changes break the tests 12.4.{5,6,8,9,10,11,13,14,15,16,17,18}.
I initially discovered this while maintaining my WebSocket libraries (https://github.com/faye), but I have reproduced the failures with the ws npm package, and with the file examples/twisted/websocket/echo_compressed/server.py in the autobahn-python repo.
The tests work fine with the 0.7.5 release. Taking the 0.7.5 package and removing the s from the end of WebSockets as is done in the above-linked commit is enough to make the test start failing.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 1
- Comments: 22 (4 by maintainers)
Commits related to this issue
- Stick to Autobahn 0.7.5, due to https://github.com/crossbario/autobahn-testsuite/issues/77. — committed to faye/wstest by jcoglan 7 years ago
- Deal with non-binary payloads as Python unicode objects. Fixes #77: - Tests which split the payload of UTF-8 encoded bytes should split using unicode rather than raw bytes. - Test data marked as "... — committed to vector-of-bool/autobahn-testsuite by vector-of-bool 6 years ago
As the person that opened this issue, and as a fellow open-source maintainer, I’d like to emphasise that shaming people for not getting a fix out within some arbitrary time period is not productive and mostly leads to maintainers burning out.
Also, the value I’ve got from this software is substantial considering I didn’t have to pay for it, and there are many thousands of projects depending on software that’s better for having this test suite. If this issue gets fixed, that’ll be a nice bonus but I’m happy to work around it.
Fun fact about public reports: I believe any implementation that currently displays the 12.5.* test cases as passing is non-compliant — it accepts broken UTF-8 that it should reject 😃
(Me from Slack) I’ve talked with @vinniefalco and I think I have a good fix for this. I’ll send in a PR when I’ve verified I haven’t broken anything. The gist of it is this:
TESTDATAmarks some test payloads as “binary,” and we deal with those as raw byte sequences. These cases weren’t having trouble because text encoding wasn’t important here.data1.htmldeal with “non-binary.” In these cases,sendOne()andonMessage()incase12_x_x.pywilldecode('utf-8')the test payload data before working with it.I believe this is the same as issue #71, eg see the comment https://github.com/crossbario/autobahn-testsuite/issues/71#issuecomment-294191346
The current code is splitting UTF8 test strings/files at non-codepoint boundaries, and the removal of the "s"es in the test file triggers the issue here. If that is all true, this is a really wicked one: the issue was always lurking under the hood, but was actually sleeping until the test data file was changed for an apparantly “typo”.
The solution is I think what I already dug out in above comment: steal the respective code from current Autobahn (which isn’t in the frozen version the test suite uses) for doing splitting that respects code point boundaries, and use that here for 12.*.