What version are you running?
What's the URL of the page containing the problem?
What steps will reproduce the problem?
- File a review request with a new file with at least two lines containing non-ASCII characters
- Create a review with two diff comments, one for each line containing non-ASCII characters
- Publish the review
What is the expected output? What do you see instead?
Expected: The page works as expected.
Actual output: One of the comments does not load at all, and the other comment's html contains some extra characters (that match the starting digits of the ID of the non-loading comment)
What operating system are you using? What browser?
Windows, on Chrome and Firefox.
Please provide any additional information below.
The root cause of this bug is a mismatch in the reported HTML length on the route
I have been able to patch on my instance that issue by replacing line 1429 with:payload.write(b'%d\n' % len(html.decode('utf-8')))
That is, I do an additional decode in order to get the real unicode character length.
Thanks for the report and the patched code. I'll get a repro case going and get this into the next release.
- New + Confirmed
Fixed on release-3.0.x (4f73586b29857839648043988b7193645e76d12f)
- Confirmed + Fixed
Thank you very much!
I will check that today, and file another bug if my suspicions are confirmed.
You're right, this is a problem. I'd like to reuse this bug for any further discussion on this, since the core problem is not truly fixed. Reopening.
- Fixed + Confirmed
So the plan was to put in the character counts instead of byte lengths as a temporary measure just for this release, so that we could ship tomorrow. However, given that it didn't really solve the problem, and given that the holidays are coming up, we're going to delay the release to the week after Christmas and do this right.
The proper solution, which I've been working on, is to retain byte strings and move to binary parsing on the client side. We'll be loading all data into an
FileReader. I have this working locally, and reliably, but it still needs additional love and testing.
I believe that this bug is fixed on 3.0.2 (I have no issues on 3.0.2, except for #4627, which was patched, and you already fixed it ;))
- Confirmed + Fixed