1213884 - Unexpected response on pushing to review, unable to create new reviews

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Andreas Tolfsen ❲:ato❳]

Unable to push to review using `hg push review`, aborting on "unexpected response: empty string".

version-control-tools is at latest tip (783405fe84cb).

Stack:

% hg push review
pushing to ssh://reviewboard-hg.mozilla.org/gecko
searching for changes
no changes found
submitting 1 changesets for review
remote: ** Unknown exception encountered with possibly-broken third-party extension pushlog-feed
remote: ** which supports versions unknown of Mercurial.
remote: ** Please disable pushlog-feed and try your action again.
remote: ** If that fixes the bug please report it to the extension author.
remote: ** Python 2.7.9 (default, Dec 12 2014, 10:25:20) [GCC 4.4.7 20120313 (Red Hat 4.4.7-11)]
remote: ** Mercurial Distributed SCM (version 3.5.1)
remote: ** Extensions loaded: pushlog, pushlog-feed, buglink, serverlog, rbserver
remote: Traceback (most recent call last):
remote:   File "/repo/hg/venv_pash/bin/hg", line 43, in <module>
remote:     mercurial.dispatch.run()
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/dispatch.py", line 30, in run
remote:     sys.exit((dispatch(request(sys.argv[1:])) or 0) & 255)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/dispatch.py", line 92, in dispatch
remote:     ret = _runcatch(req)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/dispatch.py", line 163, in _runcatch
remote:     return _dispatch(req)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/dispatch.py", line 895, in _dispatch
remote:     cmdpats, cmdoptions)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/dispatch.py", line 656, in runcommand
remote:     ret = _runcommand(ui, options, cmd, d)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/dispatch.py", line 1013, in _runcommand
remote:     return checkargs()
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/dispatch.py", line 982, in checkargs
remote:     return cmdfunc()
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/dispatch.py", line 892, in <lambda>
remote:     d = lambda: util.checksignature(func)(ui, *args, **cmdoptions)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/util.py", line 792, in check
remote:     return func(*args, **kwargs)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/commands.py", line 5713, in serve
remote:     s.serve_forever()
remote:   File "/repo/hg/version-control-tools/hgext/serverlog/__init__.py", line 332, in serve_forever
remote:     return super(sshserverwrapped, self).serve_forever()
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/sshserver.py", line 94, in serve_forever
remote:     while self.serve_one():
remote:   File "/repo/hg/version-control-tools/hgext/serverlog/__init__.py", line 361, in serve_one
remote:     return super(sshserverwrapped, self).serve_one()
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/sshserver.py", line 112, in serve_one
remote:     rsp = wireproto.dispatch(self.repo, self, cmd)
remote:   File "/repo/hg/version-control-tools/hgext/serverlog/__init__.py", line 353, in dispatch
remote:     return origdispatch(repo, proto, cmd)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/mercurial/wireproto.py", line 516, in dispatch
remote:     return func(repo, proto, *args)
remote:   File "/repo/hg/version-control-tools/hgext/reviewboard/hgrb/proto.py", line 342, in reviewboard
remote:     apikey=bzapikey)
remote:   File "/repo/hg/version-control-tools/hgext/reviewboard/hgrb/proto.py", line 92, in post_reviews
remote:     return pr(*args, **kwargs)
remote:   File "/repo/hg/version-control-tools/pylib/reviewboardmods/reviewboardmods/pushhooks.py", line 132, in post_reviews
remote:     return _post_reviews(root, repoid, identifier, commits, hgresp)
remote:   File "/repo/hg/version-control-tools/pylib/reviewboardmods/reviewboardmods/pushhooks.py", line 387, in _post_reviews
remote:     update_review_request(rr, commit)
remote:   File "/repo/hg/version-control-tools/pylib/reviewboardmods/reviewboardmods/pushhooks.py", line 230, in update_review_request
remote:     draft = rr.get_or_create_draft(**props)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/rbtools/api/decorators.py", line 27, in request_method
remote:     *args, **kwargs)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/rbtools/api/transport/sync.py", line 65, in execute_request_method
remote:     return self._execute_request(request)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/rbtools/api/transport/sync.py", line 74, in _execute_request
remote:     rsp = self.server.make_request(request)
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/rbtools/api/request.py", line 521, in make_request
remote:     content_type, body = request.encode_multipart_formdata()
remote:   File "/repo/hg/venv_pash/lib64/python2.7/site-packages/rbtools/api/request.py", line 101, in encode_multipart_formdata
remote:     content.write(self._fields[key].encode('utf-8') + NEWLINE)
remote: UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 141: ordinal not in range(128)
abort: unexpected response: empty string

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: bug_1213800.patch

Comment 2

[Mauro Doglio [:mdoglio]]

This is apparently caused by the apostrophe in the commit message. :ato was able to successfully post the review after s/’/'.
Downgrading the importance to normal because of the workaround.

Comment 3

[Gregory Szorc [:gps]]

Ummmm. Apostrophe shouldn't matter. This has a security odor to it, so marking the bug private for now.

Comment 4

[Gregory Szorc [:gps]]

Oh, I guess it was the unicode single quote (\u2019 / \xe2\x80\x99) not a normal single quote. http://www.fileformat.info/info/unicode/char/2019/index.htm. This is almost certainly just a unicode encoding bug.

This is the 2nd unicode bug I've seen today. Let me dig into this.

Comment 5

[Gregory Szorc [:gps]]

I'm able to reproduce this. Definitely a bug in encoding of the commit message as part of the push code on the server.

Comment 6

[Gregory Szorc [:gps]]

Attachment: MozReview Request: reviewboard: preserve Unicode in commit messages (bug 1213884); r?dminor

reviewboard: preserve Unicode in commit messages (bug 1213884); r?dminor

There were 2 bugs preventing Unicode in commit messages from working.

First, ctx.description() returns a mercurial.encoding.localstr instance.
This is a thin wrapper around str that quacks like a str but has the
value represented in the local encoding, which is variable and may not
be able to represent non-ASCII characters, at which point they are
represented as ASCII question marks.

Second, we were feeding this localstr or a str instance into RBTools.
RBTools appears to assume values are unicode because it blindly
performs a .encode('utf-8') on them. This operation should be illegal
(you .encode() unicode and .decode() str/bytes) but Python 2 happily
allows us.

Combined with a string concatenation, the bowels of RBTools encountered
a UnicodeDecodeError when Unicode was present in commit messages.

With this change, Unicode is now properly preserved in the Review Board
summary and description fields as well as in Bugzilla attachment names,
as the test changes demonstrate.

Comment 7

[Dan Minor [:dminor]]

Comment on attachment 8672870 [details]
MozReview Request: reviewboard: preserve Unicode in commit messages (bug 1213884); r?dminor

https://reviewboard.mozilla.org/r/21805/#review19647

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/40471e2799fbc487c3e86e94b3c670e48ee8e8af
reviewboard: preserve Unicode in commit messages (bug 1213884); r=dminor

Comment 9

[Gregory Szorc [:gps]]

This will likely get deployed today.

Comment 10

[Gregory Szorc [:gps]]

This is deployed.