gh-134759: UnboundLocalError in email.message.Message.get_payload

[skv0zsneg]

Hello!

I worked on #134759 and was able to reproduce the UnboundLocalError by passing bytes payload directly to _payload. I'm not sure can we do it in a "public" way, like throw .set_payload() but if payload some how become bytes (and content-transfer-encoding is equal quoted-printable) it is now not falling :)

I'm also add myself to ACKS list, but not sure is my changes enough for it. If you say I'll remove myself with no questions.

• Issue: UnboundLocalError in email.message.Message.get_payload #134759

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[sobolevn]

Thank you!

[picnixz]

Suggested change

	def test_get_bytes_payload_with_quoted_printable_encoding(self):
	payload = b'Some payload'
	def test_get_bytes_payload_with_quoted_printable_encoding(self):
	# See https://github.com/python/cpython/issues/134759.
	payload = b'Some payload'

[picnixz]

We can use the following for only using public methods:

        payload = memoryview(b'Some payload')
        m = self._make_message()
        m.add_header('Content-Transfer-Encoding', 'quoted-printable')
        m.set_payload(payload)
        self.assertEqual(m.get_payload(decode=True), payload)

[skv0zsneg]

Yes, I was trying to reproduce bug with "public" method .set_payload(). I'm achieved that by passing BytesIO(b"Some payload") buffer but was not sure about correct arg type of .set_payload() (typeshed want object only with the .decode() method), so came up to put bytes directly.

I'll change the way you suggest, no problem :)

[picnixz]

Yeah, decode() is not present on memoryview but it's a buffer-like protocol so I think it should be fine. However, this may be a side-effect of the current implementation :) (also, the current implementation only does duck typing, not true isinstance() checks when setting the payload)

[picnixz]

As a bugfix I think we can keep this but in a follow-up PR I think we should ensure that we are only having a list, str or bytes or handle buffer objects by wrapping them into bytes.

[skv0zsneg]

As a bugfix I think we can keep this but in a follow-up PR I think we should ensure that we are only having a list, str or bytes or handle buffer objects by wrapping them into bytes.

Got it. If nobody mind I can open an issue (or keep in #134759 ?) and take it in work :)

[picnixz]

I'm travelling for now but a separate issue for discussion would be better. It could break backwards compatibility so we should be careful.

[picnixz]

I'm also add myself to ACKS list, but not sure is my changes enough for it. If you say I'll remove myself with no questions.

IMO even contributors fixing a single typo can be added to this file so no worry!

[miss-islington-app]

Thanks @skv0zsneg for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

[bedevere-app]

GH-136579 is a backport of this pull request to the 3.14 branch.

[bedevere-app]

GH-136580 is a backport of this pull request to the 3.13 branch.