This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1080

hgweb: fix logging to use native strings as appropriate
ClosedPublic

Authored by durin42 on Oct 14 2017, 4:16 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG8782076874f5: hgweb: fix logging to use native strings as appropriate
Summary

Kind of a tangled mess, but now logging works in both Python 2 and 3.

no-check-commit because of the interface required by Python's HTTP

server code.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durin42 created this revision.Oct 14 2017, 4:16 PM
indygreg added a subscriber: indygreg.Oct 14 2017, 4:59 PM

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?

durin42 added a comment.Oct 14 2017, 9:05 PM
In 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?

yuja accepted this revision.Oct 14 2017, 11:34 PM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/hgweb/server.py
339

Replaced this with encoding.strtolocal() since socket.error may
contain platform-native string.

This revision is now accepted and ready to land.Oct 14 2017, 11:34 PM
Closed by commit rHG8782076874f5: hgweb: fix logging to use native strings as appropriate (authored by durin42). · Explain WhyOct 14 2017, 11:59 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/hgweb/server.py (27 lines)

Diff 2744

mercurial/hgweb/server.py

def preparehttpserver(httpserver, ui): def preparehttpserver(httpserver, ui):
"""Prepare .socket of new HTTPServer instance""" """Prepare .socket of new HTTPServer instance"""
def __init__(self, *args, **kargs): def __init__(self, *args, **kargs):
self.protocol_version = r'HTTP/1.1' self.protocol_version = r'HTTP/1.1'
httpservermod.basehttprequesthandler.__init__(self, *args, **kargs) httpservermod.basehttprequesthandler.__init__(self, *args, **kargs)
def _log_any(self, fp, format, *args): def _log_any(self, fp, format, *args):
fp.write("%s - - [%s] %s\n" % (self.client_address[0], fp.write(pycompat.sysbytes(
r"%s - - [%s] %s" % (self.client_address[0],
self.log_date_time_string(), self.log_date_time_string(),
format % args)) format % args)) + '\n')
fp.flush() fp.flush()
def log_error(self, format, *args): def log_error(self, format, *args):
self._log_any(self.server.errorlog, format, *args) self._log_any(self.server.errorlog, format, *args)
def log_message(self, format, *args): def log_message(self, format, *args):
self._log_any(self.server.accesslog, 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 = [] xheaders = []
if util.safehasattr(self, 'headers'): if util.safehasattr(self, 'headers'):
xheaders = [h for h in self.headers.items() xheaders = [h for h in self.headers.items()
if h[0].startswith('x-')] if h[0].startswith(r'x-')]
self.log_message('"%s" %s %s%s', self.log_message(r'"%s" %s %s%s',
self.requestline, str(code), str(size), 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): def do_write(self):
try: try:
self.do_hgweb() self.do_hgweb()
except socket.error as inst: except socket.error as inst:
if inst[0] != errno.EPIPE: if inst[0] != errno.EPIPE:
raise raise
def do_POST(self): def do_POST(self):
try: try:
self.do_write() self.do_write()
except Exception: except Exception:
self._start_response("500 Internal Server Error", []) self._start_response("500 Internal Server Error", [])
self._write("Internal Server Error") self._write("Internal Server Error")
self._done() self._done()
tb = "".join(traceback.format_exception(*sys.exc_info())) tb = r"".join(traceback.format_exception(*sys.exc_info()))
self.log_error("Exception happened during processing " # We need a native-string newline to poke in the log
"request '%s':\n%s", self.path, tb) # 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): def do_GET(self):
self.do_POST() self.do_POST()
def do_hgweb(self): def do_hgweb(self):
path, query = _splitURI(self.path) path, query = _splitURI(self.path)
env = {} env = {}
sys.setdefaultencoding(oldenc) sys.setdefaultencoding(oldenc)
address = ui.config('web', 'address') address = ui.config('web', 'address')
port = util.getport(ui.config('web', 'port')) port = util.getport(ui.config('web', 'port'))
try: try:
return cls(ui, app, (address, port), handler) return cls(ui, app, (address, port), handler)
except socket.error as inst: except socket.error as inst:
raise error.Abort(_("cannot start server at '%s:%d': %s") raise error.Abort(_("cannot start server at '%s:%d': %s")
% (address, port, inst.args[1])) % (address, port, pycompat.sysbytes(inst.args[1])))
yujaUnsubmitted
Not Done

Replaced this with encoding.strtolocal() since socket.error may
contain platform-native string.

yuja: Replaced this with encoding.strtolocal() since socket.error may contain platform-native string.