Fix email header types #1472
Fix email header types #1472
Conversation
Some email header operations can operate on or return a Header object as well as a string. Here's some sample code that demonstrates this - the Received header contains a high byte, so the header is decoded into a Header object rather than a string. (This is based on a real email and test case from the Patchwork project.) =========== from email import message_from_bytes msg=b"""From [email protected] Thu Sep 15 14:06:56 2016 Received: (from fyigit@localhost) by random.domain with \xEA id u8FC6rZs015035; Thu, 15 Sep 2016 13:06:53 +0100 From: [email protected] To: [email protected] Date: Thu, 15 Sep 2016 13:06:44 +0100 Subject: subject body """ m=message_from_bytes(msg) print(m.values()) print(m.items()) print(type(m['Received'])) print(type(m.get('Received'))) print(m.get_all('Received')) m.add_header('xyzzy', 'a') m.replace_header('xyzzy', m.get('Received')) ========== Signed-off-by: Daniel Axtens <[email protected]>
| @@ -36,16 +38,16 @@ class Message: | |||
| def get_charset(self) -> _CharsetType: ... | |||
| def __len__(self) -> int: ... | |||
| def __contains__(self, name: str) -> bool: ... | |||
| def __getitem__(self, name: str) -> Optional[str]: ... | |||
| def __setitem__(self, name: str, val: str) -> None: ... | |||
| def __getitem__(self, name: str) -> Optional[_HeaderType]: ... | |||
There was a problem hiding this comment.
Is a Union return type a good choice here? See python/mypy#1693 for the issues with returning Union.
There was a problem hiding this comment.
I've had a look at that.
My concern is that a header is very different to a string: .lower(), .strip() etc all completely fail on a Header. There is little you can correctly do with the result of get/get_all/getitem without either checking the type, running str() on it, or passing it to another function that handles either type. Otherwise you will eventually hit type errors. This hit us at the Patchwork project when a header unexpectedly contained an invalid byte and our parser broke completely.
I'm happy to look at another way to approach the problem though - this is the first time I've touched typeshed.
There was a problem hiding this comment.
@matthiaskramm - any thoughts on how else I might approach this problem?
There was a problem hiding this comment.
For what it's worth, I think a Union return type is probably the right call here based on @daxtens's arguments.
There was a problem hiding this comment.
Following up on this - any further thoughts or suggestions on how I could do this differently?
There was a problem hiding this comment.
@matthiaskramm what do you think? I'm OK with merging this as is.
There was a problem hiding this comment.
Alright, let's move forward with this.
Some email header operations can operate on or return a Header object
as well as a string.
Here's some sample code that demonstrates this - the Received header
contains a high byte, so the header is decoded into a Header object
rather than a string. (This is based on a real email and test case
from the Patchwork project.)
Signed-off-by: Daniel Axtens [email protected]