email.utils.decode_params not supporting params that contain hyphens

[mholschbach]

Bug report

Bug description:

The decode_params function from email.utils is failing to combine the parameters from multiple lines if the name of the parameter includes a hyphen "-". Maybe it's this regex which is too strict:

cpython/Lib/email/utils.py

Lines 392 to 393 in 9d1e668

	rfc2231_continuation = re.compile(r'^(?P<name>\w+)\*((?P<num>[0-9]+)\*?)?$',
	re.ASCII)

The decode_params function always ignores the first 2-tuple but the first parameter in a SMTP header can already be a multiline parameter. I don't see a reason why the first array element is skipped here:

cpython/Lib/email/utils.py

Lines 398 to 405 in 9d1e668

	params is a sequence of 2-tuples containing (param name, string value).
	"""
	new_params = [params[0]]
	# Map parameter's name to a list of continuations. The values are a
	# 3-tuple of the continuation number, the string value, and a flag
	# specifying whether a particular segment is %-encoded.
	rfc2231_params = {}
	for name, value in params[1:]:

my code to reproduce:

from email.utils import decode_params

decode_params([('parameter-name*0*',"utf-8''start;"),("parameter-name*1*","-middle-;"),("parameter-name*2*","end;")])
# parameter-name is not combined:
#  [('parameter-name*0*', "utf-8''start;"), ('parameter-name*1*', '"-middle-;"'), ('parameter-name*2*', '"end;"')]

decode_params([('parametername*0*',"utf-8''start;"),("parametername*1*","-middle-;"),("parametername*2*","end;")])
# parametername is combined but not the first 2-tuple:
# [('parametername*0*', "utf-8''start;"), ('parametername', (None, None, '"-middle-;end;"'))]

decode_params([("ignored","ignored"),('parametername*0*',"utf-8''start;"),("parametername*1*","-middle-;"),("parametername*2*","end;")])
# parametername is now combined as expected, the additional entry can be ignored when processing the output
# [('ignored', 'ignored'), ('parametername', ('utf-8', '', '"start;-middle-;end;"'))]

Thank you
Michael

CPython versions tested on:

3.12

Operating systems tested on:

Linux

[picnixz]

As you observed, adding a first dummy parameter solves the issue. Now, why do we skip the first parameter? Actually, the reason is because we are always expecting a header to be found first

cpython/Lib/email/message.py

Lines 664 to 666 in 0f20281

	value = self.get(header, missing)
	if value is missing:
	return failobj

and then we parse that header. By default, I think the first element of the list being decoded is actually the XXX of Content-Type: XXX value where no multiline support is expected (or am I wrong here?) Then, what follows contains a list of possibly multi-line parameters.

The RFC says:

A MIME disposition value may specify any number of associated parameters.
[...]
These parameter names and values end up appearing in the content-type and content-disposition header fields in Internet email.

For instance:

Content-Type: message/external-body; access-type=URL;
  URL*0="ftp://";
  URL*1="cs.utk.edu/pub/moore/bulk-mailer/bulk-mailer.tar"

Consider https://www.ietf.org/rfc/rfc2045.html#section-5.1 vs https://datatracker.ietf.org/doc/html/rfc2231#section-7. I think we always have HEADER_NAME: HEADER_VALUE[;ADDITIONAL_PARAMETERS]. Observe that:

>>> x = "Content-Type: text/html; foo=one; bar=two; baz=three\n"
>>> msg = email.message_from_string(x)
>>> msg.get_params()
[('text/html', ''), ('foo', 'one'), ('bar', 'two'), ('baz', 'three')]

More precisely:

>>> email.message._parseparam(x)
['Content-Type: text/html', 'foo=one', 'bar=two', 'baz=three']

We are actually parsing the entire header and not just its list of parameters. In particular, the "first" parameter is the header value and I suspect this is what RFC 2045 says. So the inputs to decode_params should not be manually specified but rather constructed in the same way (we still have a doc issue though IMO).

The reason why the content-type subtype is always present is because of RFC 2045 which states:

Note also that a subtype specification is MANDATORY -- it may not be
omitted from a Content-Type header field. As such, there are no
default subtypes.

EDIT: Note that the RFC only says something about the Content-Type header field. For other header fields without this, we may have some issues depending on how you actually specify a header without a value but with a parameter. If you add a ; to separate the (empty) value from the list of parameters, it's fine. Otherwise, there will be parsing issues, though I don't think they are common because a header without a value is not common (is it even legal? I'm not sure that it is actually).

---

Now, let's move onto the second issue, namely the -. I think it's incorrect to reject them because neither RFC 2045 nor RFC 2231 say that the character is illegal. The ABNF is:

attribute := 1*attribute-char
attribute-char := <any (US-ASCII) CHAR except SPACE, CTLs,
                 "*", "'", "%", or tspecials>

and - is not part of tspecials which is (documented in RFC 2045)

tspecials :=  "(" / ")" / "<" / ">" / "@" /
             "," / ";" / ":" / "\" / <">
             "/" / "[" / "]" / "?" / "="

---

So, AFAICT, the title of the issue is correct, but there shouldn't be an issue with the implementation that skips the first parameter. There is however an issue with the documentation that should mention that the first element of the parameters list is skipped as the input should be parsed from a valid RFC 2045 header value.

[mholschbach]

Thank you for the detailed response.

While reading the email.message documentation yesterday I saw that the functions to manage params were focussed on Content-Type and Content-Dispostion header fields. The header field I want to parse indeed looks like this:

myheader: my-param*0="base64";
  my-param*1="morebase64";
  my-param*2="evenmorebase64";

I will have a look at the RFCs if we need to have a value before the first parameter. For now I'm calling decode_params directly anyway and can add a dummy entry which I ignore later on.

[mholschbach]

I couldn't find a RFC that mandated a value in front of the first parameter like the Content-Type and other header definitions:
content := "Content-Type" ":" type "/" subtype *(";" parameter)

I found a few proprietary headers that skip the value and instead start with a multi-line parameter after the header name. Typical candidates in the wild are proxies that add their results from scanning for malware, spam etc. in the form of base64 encoded data into the mail header.

Another example I found is from https://en.wikipedia.org/wiki/DomainKeys_Identified_Mail
DKIM-Signature: v=1; a=rsa-sha256; d=example.net; s=brisbane; c=relaxed/simple; q=dns/txt; i=foo@eng.example.net; t=1117574938; x=1118006938; l=200; h=from:to:subject:date:keywords:keywords; z=From:foo@eng.example.net|To:joe@example.com| Subject:demo=20run|Date:July=205,=202005=203:44:08=20PM=20-0700; bh=MTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0NTY3ODkwMTI=; b=dzdVyOfAKCdLXdJOc9G2q8LoXSlEniSbav+yuU4zGeeruD00lszZ VoG4ZHRNiYzR

msg.get_params(header="DKIM-Signature")
works as expected for this input and the version parameter will probably never require multiple lines. But if the first line is changed to
DKIM-Signature: v*0=1; a*0=rsa-sha256; d=example.net; s=brisbane;
then only signing algorithm a is parsed correctly, but not the version v.

[picnixz]

Actually, decode_params() is said to decode parameters in the sense of RFC 2231, right? And this RFC is focused on Content-Type and Content-Disposition:

This memo defines extensions to the RFC 2045 media type and RFC 2183 disposition parameter value mechanisms to provide [...]

If I were to be pedantic, we could say that those two should only be used for those. However, as you mentioned, we could have a more generic way of doing it. However, I'm not comfortable for changing the behaviour of decode_params because it could cause breaking changes. But at the same time it's kind of a bug because it doesn't say that the first item is ignored.

Concerning DKIM signature, the RFC says: https://www.ietf.org/rfc/rfc6376.html#section-3.5

The DKIM-Signature value is a tag-list as described in Section 3.2

so it doesn't necessarily fall under RFC 2231, hence I'm not sure RFC 2231 applies here.

As this now exceeds my knowledge, I'd like to ask experts in @python/email-team for their recommendations.

[bitdancer]

Right, decode_params is part of the old API, and is not very friendly. Yes, it would be nice to document the fact that the first tuple passed in is ignored. Truthfully decode_params was never intended for public use; I'm not sure why it is even documented. You'll note that it is only called from a private method in the library itself. And yes, when it was implemented, the only headers that existed, even in the wild, were the ones referred to in RFC 2331.

I do not think that we should change decode_params.

As for what to do about new headers that support mime style parameters, including headers that do not have a value, only parameters: the new email API supports that, though not without some configuration. This will solve your problem:

from email import message_from_bytes
from email.policy import default
from email import headerregistry as registry
from email import _header_value_parser as parser


class myheader(registry.ParameterizedMIMEHeader):

    value_parser = staticmethod(parser.parse_mime_parameters)


myregistry = registry.HeaderRegistry()
myregistry.map_to_type('myheader', myheader)
mypolicy = default.clone(header_factory=myregistry)

m = message_from_bytes(b"""\
myheader: my-param*0="base64";
  my-param*1="morebase64";
  my-param*2="evenmorebase64";
""", policy=mypolicy)

print(m['myheader'])
print(m['myheader'].params)

 my-param="base64morebase64evenmorebase64"
{'my-param': 'base64morebase64evenmorebase64'}

As you note, however, DKIM exists, and we really should add direct support for it to headerregistry. That's issue #79062.

This issue should be closed by the doc fix discussed above.

But it would be nice to add something like the above example to the examples chapter, since it is far from obvious. However, you'll note that the above code is using a 'private' module and one of its internal functions...the intent was to make the value parser into a public API once it had been refined a bit. Since I've never had time to do that, and it seems to be working well enough, it may be time to start thinking about documenting it...but that's a much bigger issue.

What we could do to dodge that problem for the present would be to add a 'ParametersOnlyMIMEHeader' class with the definition given to myheader above to headerregistry. Then the example would be just showing how to declare a custom header to be of that type by adding it to a custom registry. We could make that a new issue, and then also, as interim step before full support for DKIM, declare the DKIM headers to be of this new type.

[picnixz]

I do not think that we should change decode_params.

Couldn't we change it at least to support -? because we're not really RFC-friendly then =/

If we don't want to change it, we should at least amend the current docs.