[issue47047] smtplib: allow custom policy or use msg.policy in send_message

2022-03-17 Thread Mikael Koli


Mikael Koli  added the comment:

It seems the message's policy is actually used. However, the mangle_from_ is 
still always True as the policy is not passed in the initiation of the 
generator. It seems though that all the options I mentioned could still make 
the mangle_from_ to be changeable if one wishes so.

--

___
Python tracker 
<https://bugs.python.org/issue47047>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue47047] smtplib: allow custom policy or use msg.policy in send_message

2022-03-17 Thread Mikael Koli


New submission from Mikael Koli :

The method smtplib.SMTP.send_message does not use the message's Policy if all 
of the from_addrs or to_addrs are not international. See: 
https://github.com/python/cpython/blob/v3.10.3/Lib/smtplib.py#L983 (unchanged 
in current main). The email.generator.BytesGenerator does not capture the 
email's policy as it was not passed to its init.

This has at least one notable setback: you cannot set the mangle_from to False 
meaning that the method will always turn "From ..." to ">From ..." in the plain 
text part (though often that is desirable). This is especially confusing as 
email library has the mangle_from as False by default for EmailMessages but 
smtplib.SMTP's send_message does not respect this by default.

The smtplib.SMTP.send_message has a mention about this in the docstring thus 
not entirely sure if intentional:

... Otherwise the generator is called without modifying the
policy.


If we changed this line: 
https://github.com/python/cpython/blob/v3.10.3/Lib/smtplib.py#L983

from this:
g = email.generator.BytesGenerator(bytesmsg)

to this:
g = email.generator.BytesGenerator(bytesmsg, policy=msg.policy.clone()

smptlib's tests are passed but I suspect it's not that simple. The docstring 
mention indicates this is at some level intentional and I think the mangle_from 
needs to remain True as otherwise, it may cause security problems in existing 
code. Another option perhaps could be that the policy could be passed with the 
send_message and that is used if not None or we could have argument 
"msg_policy=False" that if True, the message's policy is used.

One could also think that this could be overcome by subclassing the SMTP. 
However, the logic is such deep in that it is not convenient.

So in short, the options I thought of:
- Have an argument "policy" in send_message to force usage of your own policy 
(safe option)
- Have an argument "msg_policy" (name debatable) in send_message and if True, 
the message's policy is always used (safe option)
- Use the message's policy always (unsafe, possibly breaking and causing 
security issues in existing code)

--
components: Library (Lib), email
messages: 415428
nosy: Miksus, barry, r.david.murray
priority: normal
severity: normal
status: open
title: smtplib: allow custom policy or use msg.policy in send_message
versions: Python 3.10, Python 3.11, Python 3.7, Python 3.8, Python 3.9

___
Python tracker 
<https://bugs.python.org/issue47047>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue44924] logging.handlers.QueueHandler does not maintain the exc_text

2021-08-16 Thread Mikael Koli


Mikael Koli  added the comment:

I did subclass the QueueHandler and it did circumvent the issue for me. But 
after spending some time to find out why I could not access the exc_text I 
thought maybe there is something that could be done to make it more intuitive 
and save others' time. I find the current behaviour somewhat confusing.

--

___
Python tracker 
<https://bugs.python.org/issue44924>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue44924] logging.handlers.QueueHandler does not maintain the exc_text

2021-08-16 Thread Mikael Koli


Mikael Koli  added the comment:

And the reason why overriding attribute 'record.msg' with the formatted message 
is problematic is that the method 'record.getMessage' (which is used by 
Formatter) fetches the log message from the variable 'record.msg'. Therefore 
the exc_text needs to be set to None as Formatter checks the existence of this 
attribute to decide whether or not to format the exception. Otherwise the 
exception text may be formatted multiple of time.

I think it may be more correct solution to not override the record.msg and let 
the other formatters down the queue reformat the message if required.

--

___
Python tracker 
<https://bugs.python.org/issue44924>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue44924] logging.handlers.QueueHandler does not maintain the exc_text

2021-08-16 Thread Mikael Koli


New submission from Mikael Koli :

The reason why logging.handlers.QueueHandler does not maintain exc_text is 
obvious:

def prepare(self, record):
...
record = copy.copy(record)
record.message = msg
record.msg = msg
record.args = None
record.exc_info = None
record.exc_text = None
return record

The record.exc_text is set to None. The reason for this is to prevent the 
exception text showing up multiple times to the message. See 
https://bugs.python.org/issue34334.

However, there are a couple of situations this may cause a problem. First, it's 
not currently possible to format the exception of the record in a handler on 
the other side of the queue. Second, it's not possible to let the handler on 
the other side of the queue utilize exc_text. The default handlers do not 
behave in such a way but one could prefer to create their own handler that does 
so, such as log the records to a database with a column for the exception text.


Possible solution:
Don't override the record.msg and don't set the record.exc_text to None. I 
think it could be done simply:

def prepare(self, record):
...
record = copy.copy(record)
record.message = msg
# record.msg = msg
record.args = None
record.exc_info = None
# record.exc_text = None
return record

 
This way one can format the record later again without multiple exception text 
showing up in the message. Doing so will fail the test 
'test_logging.QueueHandlerTest.test_formatting' as this tests the record.msg is 
the same as record.message. This may cause issues if someone relies on 
record.msg. On the other hand, now other formatters and handlers down the line 
could use the exc_text attribute. I'm not sure if this is too breaking change 
or not.

The failing test:

def test_formatting(self):
msg = self.next_message()
levelname = logging.getLevelName(logging.WARNING)
log_format_str = '{name} -> {levelname}: {message}'
formatted_msg = log_format_str.format(name=self.name,
  levelname=levelname, message=msg)
formatter = logging.Formatter(self.log_format)
self.que_hdlr.setFormatter(formatter)
self.que_logger.warning(msg)
log_record = self.queue.get_nowait()
self.assertEqual(formatted_msg, log_record.msg) # 
self.assertEqual(formatted_msg, log_record.message)



I tested this issue with the following test (which is a pass with the current 
build):

class QueueHandlerTest(BaseTest):

def test_formatting_exc_text(self):
formatter = logging.Formatter(self.log_format)
self.que_hdlr.setFormatter(formatter)
try:
raise RuntimeError('deliberate mistake')
except:
self.que_logger.exception('failed', stack_info=True)
log_record = self.queue.get_nowait()
self.assertTrue(log_record.exc_text.startswith('Traceback (most recent '
   'call last):\n'))

--
components: Library (Lib)
messages: 399642
nosy: Miksus
priority: normal
severity: normal
status: open
title: logging.handlers.QueueHandler does not maintain the exc_text
type: behavior
versions: Python 3.10, Python 3.11, Python 3.6, Python 3.7, Python 3.8, Python 
3.9

___
Python tracker 
<https://bugs.python.org/issue44924>
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com