| Submitter | phabricator |
|---|---|
| Date | Oct. 14, 2017, 8:16 p.m. |
| Message ID | <differential-rev-PHID-DREV-aingnqtfrbersxp3im2g-req@phab.mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/24911/ |
| State | Superseded |
| Headers | show |
Comments
indygreg added a comment. I'm not crazy about not logging bytes everywhere. Why is the file-like object taking bytes in Python 2 but Unicode in Python 3? It feels like you stumbled onto an existing bug where the logging mechanism isn't encoding aware? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1080 To: durin42, #hg-reviewers Cc: indygreg, mercurial-devel
durin42 added a comment.
In https://phab.mercurial-scm.org/D1080#18100, @indygreg wrote:
> I'm not crazy about not logging bytes everywhere. Why is the file-like object taking bytes in Python 2 but Unicode in Python 3? It feels like you stumbled onto an existing bug where the logging mechanism isn't encoding aware?
The file-like object to which we're logging is always in bytes mode - we traffic in unicodes from the callsite of log_error and log_request because there's lots of internal wsgi bits that are unicodes, and it's easier to make the log template a native string and bounce it down to bytes on Python 3 than to try and wrap the entire WSGI universe in a bytes blanket. Does that clarify things?
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1080
To: durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
yuja accepted this revision. yuja added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > server.py:339 > raise error.Abort(_("cannot start server at '%s:%d': %s") > - % (address, port, inst.args[1])) > + % (address, port, pycompat.sysbytes(inst.args[1]))) Replaced this with encoding.strtolocal() since socket.error may contain platform-native string. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1080 To: durin42, #hg-reviewers, yuja Cc: yuja, indygreg, mercurial-devel
Patch
diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py --- a/mercurial/hgweb/server.py +++ b/mercurial/hgweb/server.py @@ -67,25 +67,26 @@ httpservermod.basehttprequesthandler.__init__(self, *args, **kargs) def _log_any(self, fp, format, *args): - fp.write("%s - - [%s] %s\n" % (self.client_address[0], - self.log_date_time_string(), - format % args)) + fp.write(pycompat.sysbytes( + r"%s - - [%s] %s" % (self.client_address[0], + self.log_date_time_string(), + format % args)) + '\n') fp.flush() def log_error(self, format, *args): self._log_any(self.server.errorlog, format, *args) def log_message(self, format, *args): self._log_any(self.server.accesslog, format, *args) - def log_request(self, code='-', size='-'): + def log_request(self, code=r'-', size=r'-'): xheaders = [] if util.safehasattr(self, 'headers'): xheaders = [h for h in self.headers.items() - if h[0].startswith('x-')] - self.log_message('"%s" %s %s%s', + if h[0].startswith(r'x-')] + self.log_message(r'"%s" %s %s%s', self.requestline, str(code), str(size), - ''.join([' %s:%s' % h for h in sorted(xheaders)])) + r''.join([r' %s:%s' % h for h in sorted(xheaders)])) def do_write(self): try: @@ -101,9 +102,13 @@ self._start_response("500 Internal Server Error", []) self._write("Internal Server Error") self._done() - tb = "".join(traceback.format_exception(*sys.exc_info())) - self.log_error("Exception happened during processing " - "request '%s':\n%s", self.path, tb) + tb = r"".join(traceback.format_exception(*sys.exc_info())) + # We need a native-string newline to poke in the log + # message, because we won't get a newline when using an + # r-string. This is the easy way out. + newline = chr(10) + self.log_error(r"Exception happened during processing " + r"request '%s':%s%s", self.path, newline, tb) def do_GET(self): self.do_POST() @@ -331,4 +336,4 @@ return cls(ui, app, (address, port), handler) except socket.error as inst: raise error.Abort(_("cannot start server at '%s:%d': %s") - % (address, port, inst.args[1])) + % (address, port, pycompat.sysbytes(inst.args[1])))