Skip to content

Conversation

mukeshpanchal27
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 commented Oct 19, 2025

Trac ticket: https://core.trac.wordpress.org/ticket/43258

Follow-up to #10293

This pull request refines the way the wp_finalize_template_enhancement_output_buffer function parses and checks HTTP headers to determine the content type. The main focus is on making the header parsing more robust and accurate.

Improvements to header parsing:

  • Improved parsing of HTTP headers by trimming whitespace and converting the header name to lowercase only (instead of the entire header line), ensuring accurate detection of the Content-Type header.
  • Added additional checks to skip malformed headers and continue processing, making the function more resilient to unexpected header formats.

Code cleanup:

  • Fixed a minor code indentation issue by removing an unnecessary closing brace.

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@mukeshpanchal27 mukeshpanchal27 marked this pull request as ready for review October 19, 2025 05:52
Copy link

github-actions bot commented Oct 19, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props mukesh27, westonruter.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

continue;
}

$name = strtolower( trim( $header_parts[0] ) );
Copy link
Member

Choose a reason for hiding this comment

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

Trimming of the header name was intentionally removed in accordance of the HTTP spec

Copy link
Member Author

Choose a reason for hiding this comment

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

In that case, can we add an inline comment explaining this behaviour so it doesn’t come up again in future reviews? Just curious what’s the reason behind this difference?

Copy link
Member

Choose a reason for hiding this comment

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

The rationale is in this thread: #10293 (comment)

@westonruter
Copy link
Member

I'm not sure this is more refined? Now there are multiple continue statements.

@mukeshpanchal27
Copy link
Member Author

I'm not sure this is more refined? Now there are multiple continue statements.

Is there any drawback to using multiple continue statements?

@westonruter
Copy link
Member

It's more verbose, less concise.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants