Skip to content

Conversation

rossabaker
Copy link
Member

As per RFC6266, Section 4.3, if filename* is defined, prefer that. If not, try filename.

Kind of like #6475, but lossless.

@rossabaker rossabaker requested a review from danicheg June 21, 2022 21:34
@mergify mergify bot added series/0.22 PRs targeting 0.22.x module:core labels Jun 21, 2022
Copy link
Member

@danicheg danicheg left a comment

Choose a reason for hiding this comment

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

The notable differences between the two approaches:

  1. Users that parse filename from the parameters manually will get new behavior without changes in their code (if we will do things in parser*).
  2. Doing 'magic' within the parser will allow us to soft parsing rules for filename in case of the presence of filename*. To fix this issue #5809, for example. I was planning to dig into it further. But probably we are doing well in strict behavior. It's needed to clarify.

Nevertheless, I like this PR. It seems more transparent and less tricky, to be honest. So it's up to you to decide where we go with this.

@rossabaker
Copy link
Member Author

Yeah, I like preserving both parameters as they came to us, but giving a method that gives an interpretation that steers toward proper interpretation of the spec. What to do about parsing non-Latin-1 in filename is one of those irksome strictness questions that I don't think changes what we do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:core series/0.22 PRs targeting 0.22.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants