Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.10] gh-68966: Make mailcap refuse to match unsafe filenames/types/params (GH-91993) #93543

Open
wants to merge 1 commit into
base: 3.10
Choose a base branch
from

Conversation

miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Jun 6, 2022

(cherry picked from commit b9509ba)

Co-authored-by: Petr Viktorin encukou@gmail.com

…arams (pythonGH-91993)

(cherry picked from commit b9509ba)

Co-authored-by: Petr Viktorin <encukou@gmail.com>
@encukou
Copy link
Member

@encukou encukou commented Jun 6, 2022

@gpshead said:

The failure scenario for an application where we simply start claiming no match (return None?) on potentially suspicious filenames seems acceptable. I doubt it would be a big deal to users if any even notice at all. So if you want to go forward with a PR like #91993 my gut feeling is that nobody is going to balk at the change, even in security only release branches.

@pablogsal, do you agree?

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Jun 6, 2022

@gpshead said:

The failure scenario for an application where we simply start claiming no match (return None?) on potentially suspicious filenames seems acceptable. I doubt it would be a big deal to users if any even notice at all. So if you want to go forward with a PR like #91993 my gut feeling is that nobody is going to balk at the change, even in security only release branches.

@pablogsal, do you agree?

I agree, but I am still not confident on backporting it, so unless there is some clear consensus from everyone I would recommend to be cautious here.

@miss-islington
Copy link
Contributor Author

@miss-islington miss-islington commented Jun 6, 2022

Status check is done, and it's a success .

@encukou
Copy link
Member

@encukou encukou commented Jun 6, 2022

Who's "everyone"?
Not too many people are interested in mailcap :)

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Jun 6, 2022

Everyone is any core Dev interested on mailcap that want to voice their opinion. If nobody objects or everyone is just you and @gpshead then go ahead and merge it :)

@encukou
Copy link
Member

@encukou encukou commented Jun 9, 2022

So let's ping core devs from the original issue – but I doubt even they are particularly interested in mailcap.

@zooba @brettcannon @vstinner, do you agree with Greg?

The failure scenario for an application where we simply start claiming no match (return None?) on potentially suspicious filenames seems acceptable. I doubt it would be a big deal to users if any even notice at all. So if you want to go forward with a PR like [this] my gut feeling is that nobody is going to balk at the change, even in security only release branches.

@@ -60,6 +60,18 @@ standard. However, mailcap files are supported on most Unix systems.
use) to determine whether or not the mailcap line applies. :func:`findmatch`
will automatically check such conditions and skip the entry if the check fails.

.. versionchanged:: 3.11
Copy link
Member

@gpshead gpshead Jun 9, 2022

Choose a reason for hiding this comment

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

update with the 3.10.x version number for this branch.

@brettcannon
Copy link
Member

@brettcannon brettcannon commented Jun 9, 2022

So let's ping core devs from the original issue – but I doubt even they are particularly interested in mailcap.

Correct, I am not interested and thus have no opinion. 😁

@zooba
Copy link
Member

@zooba zooba commented Jun 9, 2022

My only strong preference is that there should be a clear what's new entry that specifies at least the name of the warning, to give users encountering this the best possible chance to figure out what has changed (and that it was us, and not something that they did).

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

Successfully merging this pull request may close these issues.

None yet

7 participants