email.utils.collapse_rfc2231_value raises AttributeError on non-3-tuples

[stefanzetzsche]

Bug report

Bug description:

Issue

collapse_rfc2231_value raises AttributeError: 'tuple' object has no attribute 'startswith' when passed a tuple whose length is not 3. The guard if not isinstance(value, tuple) or len(value) != 3 is meant to handle this case, but falls through to unquote(value) which expects a string.

Reproducer

from email.utils import collapse_rfc2231_value
collapse_rfc2231_value(('a', 'b'))
# AttributeError: 'tuple' object has no attribute 'startswith'

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

• gh-145824: email.utils: replace AttributeError with TypeError in collapse_rfc2231_value for non-string non-3-tuple input #145825

[picnixz]

I don't think we should change the behavior here. First, it's not a crash but rather a GIGO case (garbage in, garbage out). The documentation says:

For convenience, if the value passed to collapse_rfc2231_value() is not a tuple, it should be a string and it is returned unquoted.

So returning the stringification of the tuple may not necessarily be correct. Instead, a TypeError should be raised if AttributeError is too cryptic IMO.

[stefanzetzsche]

Thanks for the feedback! I appreciate this is a minor issue. I agree that a TypeError would be better than the AttributeError here.

As for the documentation:

(...) may return a 3-tuple (...) :func:collapse_rfc2231_value turns this into a unicode string.

For convenience, if the value passed to collapse_rfc2231_value() is not a tuple, it should be a string and it is returned unquoted.

It currently leaves implicit/undefined the case when the value passed is a tuple, but not a 3-tuple. I'm assuming "not a tuple" already means "not a 3-tuple" given the context, but making it explicit wouldn't hurt.

I'll revise the PR accordingly.

[picnixz]

Yeah, for me it was obvious that the "this" in "this into a unicode string" referred to "a 3-tuple" returned by the other function. I'd be more in favor of (1) having a TypeError (2) and a doc clarification if necessary.