Skip to content

gh-73065: Add Date header if missing in smtplib send_message - #136850

Open
Yoav11 wants to merge 17 commits into
python:mainfrom
Yoav11:bpo-28879
Open

Yoav11 wants to merge 17 commits into
python:mainfrom
Yoav11:bpo-28879

Conversation

@Yoav11

@Yoav11 Yoav11 commented Jul 20, 2025

Copy link
Copy Markdown
Contributor

Description

this PR was created during EuroPython 2025 Sprint Weekend.

It builds on #5176 authored by @elafontaine


📚 Documentation preview 📚: https://cpython-previews--136850.org.readthedocs.build/

@python-cla-bot

python-cla-bot Bot commented Jul 20, 2025

Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

CLA not signed

@Yoav11 Yoav11 changed the title gh-28879: Add Date header if missing in smtplib send_message bpo-28879: Add Date header if missing in smtplib send_message Jul 20, 2025
@StanFromIreland

Copy link
Copy Markdown
Member

Please change the title to use the GitHub issue number.

@Yoav11 Yoav11 changed the title bpo-28879: Add Date header if missing in smtplib send_message gh-73065: Add Date header if missing in smtplib send_message Jul 20, 2025
@Yoav11
Yoav11 marked this pull request as ready for review July 20, 2025 08:22
@Yoav11
Yoav11 requested a review from a team as a code owner July 20, 2025 08:22
Comment thread Doc/library/smtplib.rst Outdated
Comment thread Lib/smtplib.py Outdated
@tomasr8 tomasr8 added the sprint label Jul 20, 2025
Comment thread Lib/test/test_smtplib.py Outdated
Comment thread Lib/test/test_smtplib.py
Comment thread Lib/smtplib.py Outdated
Comment thread Lib/test/test_smtplib.py Outdated
Comment thread Doc/library/smtplib.rst Outdated
Comment thread Lib/test/test_smtplib.py Outdated
Comment thread Misc/NEWS.d/next/Library/2025-07-20-09-51-25.bpo-28879.SLDgcy.rst Outdated
@StanFromIreland

Copy link
Copy Markdown
Member

@Yoav11 A little suggestion, when you complete a suggestion, whether that be by implementing it or providing a reason why you are not going to, please mark the conversation as "Resolved," this keeps the PR more organized.

@ilovelinux ilovelinux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider mocking the date with a non-UTC time of which we know the timezone

Comment thread Lib/test/test_smtplib.py
@StanFromIreland

Copy link
Copy Markdown
Member

Please do not use the Update Branch button unless necessary (e.g. fixing conflicts, jogging the CI, or very old PRs) as it uses valuable resources. For more information see the devguide.

@ilovelinux ilovelinux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Well done

Comment thread Lib/test/test_smtplib.py
@ilovelinux

Copy link
Copy Markdown
Contributor

@StanFromIreland the PR should be ready to be merged

@ilovelinux

Copy link
Copy Markdown
Contributor

@StanFromIreland bump 🙂

@StanFromIreland

Copy link
Copy Markdown
Member

The email experts have been requested, please be patient.

@ilovelinux

Copy link
Copy Markdown
Contributor

Thanks for the update 🙂

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 27, 2026
@github-project-automation github-project-automation Bot moved this to Todo in Sprint Apr 27, 2026

@zware zware left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM with an optional cleanup nit and a question.

Comment thread Lib/smtplib.py Outdated
raise ValueError("message has more than one 'Resent-' header block")

# RFC 5322 section 3.6, 4th Paragraph
if msg.get('Date', None) is None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naive question: would it make any sense for a user to have explicitly set msg['Date'] = None? Should we instead check 'Date' in msg?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, I would assume that if you want "Date" to appear but without anything that you would set it to an empty string ("") and not 'None'.

Setting it to None doesn't make sense to me as this is the value on default argument to say "it might not be passed through".

However, i agree that it would be safer.

Comment thread Lib/test/test_smtplib.py Outdated
timeout=support.LOOPBACK_TIMEOUT)
self.addCleanup(smtp.close)
self.assertEqual(smtp.send_message(msg), {})
""".format(date))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think it would be fine to go with an f-string here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this back in 2017 i think... f-string didn't exist 😆.

@github-project-automation github-project-automation Bot moved this from Todo to In Progress in Sprint Jul 18, 2026
@zware zware removed the stale Stale PR or inactive for long period of time. label Jul 18, 2026
@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #34682929 | 📁 Comparing b333af2 against main (02fae7a)

  🔍 Preview build  

2 files changed
± library/smtplib.html
± whatsnew/changelog.html

@Yoav11
Yoav11 requested a review from elafontaine September 21, 2026 20:32
@Yoav11

Yoav11 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

@elafontaine I believe your account still needs to sign the CLA ?

@elafontaine

Copy link
Copy Markdown

@Yoav11 , I signed back in 2020

image

So unless something changed ?

@zware

zware commented Sep 22, 2026

Copy link
Copy Markdown
Member

There's a different email associated with the commit in this PR; see the latest version of the cla-bot comment.

@Yoav11

Yoav11 commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

@elafontaine do you still have access to the email mailto:elafonta@bell.ca ? If not, @zware do you have a suggestion ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

7 participants