This is an archive of the discontinued Mercurial Phabricator instance.

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?

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
340	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

			Path	Packages
M			mercurial/hgweb/server.py (28 lines)

Status	Author	Revision
Closed	durin42	D891 cleanup: use urllibcompat for renamed methods on urllib request objects
Closed	durin42	D935 urllibcompat: move some adapters from pycompat to urllibcompat
Closed	durin42	D890 urllibcompat: new library to help abstract out some python3 urllib2 stuff
Closed	durin42	D889 url: use native strings for header values
Closed	durin42	D888 keepalive: python 3 portability tweaks
Closed	durin42	D1067 python3: use our bytes-only version of cgi.escape everywhere
Closed	durin42	D1066 url: add cgi.escape equivalent for bytestrings
Closed	durin42	D1065 python3: move from using func_name to __name__
Closed	durin42	D1080 hgweb: fix logging to use native strings as appropriate
Closed	durin42	D1079 hgweb: rewrite most obviously-native-strings to be native strings
Closed	durin42	D1078 hgweb: use native strings consistently for querystring parsing
Closed	durin42	D1077 hgweb: rewrite two X or Y and Z ad-hoc ternaries with Y if X else Z
Closed	durin42	D1076 hgweb: more native string treatment in query string parsing
Closed	durin42	D1075 httppeer: pass url to urllib as native str, not bytes
Closed	durin42	D887 httppeer: use native strings for headers
Closed	durin42	D886 ui: convert to/from Optional[bytes] to Optional[str] in password manager
Closed	durin42	D885 httppasswordmgrdbproxy: specify exact arguments

Diff 2760

mercurial/hgweb/server.py

	import os			import os
	import socket			import socket
	import sys			import sys
	import traceback			import traceback

	from ..i18n import _			from ..i18n import _

	from .. import (			from .. import (
				encoding,
	error,			error,
	pycompat,			pycompat,
	util,			util,
	)			)

	httpservermod = util.httpserver			httpservermod = util.httpserver
	socketserver = util.socketserver			socketserver = util.socketserver
	urlerr = util.urlerr			urlerr = util.urlerr
	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, encoding.strtolocal(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.

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	2744		Oct 14 2017, 4:16 PM	★	★
Diff 2	2760	rHG8782076874f5ebf5bd7f07e4ea69a7ef4e0fe74c	Oct 14 2017, 3:43 PM	★	★

This is an archive of the discontinued Mercurial Phabricator instance.

hgweb: fix logging to use native strings as appropriateClosedPublic

Details

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

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 2760

mercurial/hgweb/server.py

hgweb: fix logging to use native strings as appropriate
ClosedPublic

Revision Contents
Changeset List