Add Content-Type directive to body factory .body_factory_info by bryancall · Pull Request #12947 · apache/trafficserver

bryancall · 2026-03-06T22:31:29Z

Problem

Body factory error responses are always served with Content-Type: text/html and there is no way to change this. The .body_factory_info metadata file supports Content-Language and Content-Charset directives but has no Content-Type directive. Operators who serve non-HTML error pages (plain text, JSON, etc.) have no mechanism to set the correct MIME type.

Additionally, HttpSM::setup_internal_transfer unconditionally overwrites Content-Type to text/html when internal_msg_buffer_type is NULL, which would mask any type set earlier in the response pipeline.

Summary

Add Content-Type directive to .body_factory_info -- Operators can now set the MIME type for body factory error responses (e.g. Content-Type: text/plain) instead of the hardcoded text/html default. The directive is parsed alongside the existing Content-Language and Content-Charset directives.
Fix Content-Type overwrite in HttpSM::setup_internal_transfer -- Now uses the MIME_PRESENCE_CONTENT_TYPE presence bit to only apply the text/html default when Content-Type is not already present in the response.
Add documentation for .body_factory_info metadata file -- The admin guide's error messages page now documents all three supported directives (Content-Language, Content-Charset, Content-Type) with examples.
Add autest -- New body_factory_content_type.test.py verifies both default (text/html) and custom (text/plain) Content-Type behavior.

Testing

Built with ASAN, tested manually (default and custom Content-Type verified via curl)
Autest passes: both default and custom Content-Type scenarios

Fixes: #10893

The body factory previously only supported Content-Language and Content-Charset directives in .body_factory_info. This adds a Content-Type directive so operators can control the MIME type of error responses (e.g. text/plain instead of the default text/html). Also fixes HttpSM::setup_internal_transfer which was unconditionally overwriting Content-Type to text/html, masking any type set by the body factory. Now uses the presence bit check to only set the default when Content-Type is not already present in the response.

The :file: role with nitpicky mode causes an unresolved reference warning for .body_factory_info. Use literal markup instead.

Copilot

Pull request overview

This PR extends Apache Traffic Server’s body factory error-page customization to support a configurable Content-Type via .body_factory_info, and prevents HttpSM from overwriting an already-set Content-Type during internal transfers.

Changes:

Add parsing/storage of a new Content-Type directive in .body_factory_info and use it when generating body-factory responses.
Update HttpSM::setup_internal_transfer() to only apply the text/html default when Content-Type is not already present.
Add documentation and a gold test covering both default and custom Content-Type behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/gold_tests/body_factory/body_factory_content_type.test.py	New autest validating default (`text/html`) vs custom (`text/plain`) body-factory `Content-Type`.
src/proxy/http/HttpSM.cc	Avoids unconditional `Content-Type: text/html` overwrite by checking the presence bit.
src/proxy/http/HttpBodyFactory.cc	Parses `Content-Type` from `.body_factory_info` and incorporates it into generated response headers.
include/proxy/http/HttpBodyFactory.h	Extends internal `fabricate()` signature and body set metadata to include `content_type`.
doc/admin-guide/monitoring/error-messages.en.rst	Documents `.body_factory_info` directives, including the new `Content-Type`.
configs/body_factory/default/.body_factory_info	Updates shipped template metadata comments to include the new directive.

You can also share your feedback on Copilot code review. Take the survey.

doc/admin-guide/monitoring/error-messages.en.rst

src/proxy/http/HttpBodyFactory.cc

- Rename type_ptr to content_type_ptr to avoid confusion with the type parameter (error template name) - Fix docs: .body_factory_info is required for a template set to load, not optional. Absent file causes the directory to be skipped. - Use literal markup for .body_factory_info to fix Sphinx nitpick warning

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

You can also share your feedback on Copilot code review. Take the survey.

src/proxy/http/HttpBodyFactory.cc

tests/gold_tests/body_factory/body_factory_content_type.test.py

doc/admin-guide/monitoring/error-messages.en.rst

bryancall · 2026-03-16T15:49:29Z

[approve ci autest 0 autest 1 autest 2 autest 3]

Only include "; charset=X" when a custom Content-Type is configured in .body_factory_info. Default responses emit just "text/html" to preserve backward-compatible header behavior.

Avoid duplicating charset when a custom body-factory Content-Type already sets one, normalize Korean language-tag examples to BCP47, and make the autest Content-Type matching more formatting-tolerant.

bryancall · 2026-03-23T23:52:01Z

[approve ci autest 3]

The body factory Content-Type feature changes how error page headers are generated: charset is no longer appended when no explicit Content-Type directive exists, and setup_internal_transfer now preserves existing Content-Type instead of always overwriting with text/html.

Use internal_msg_buffer_type to carry the body factory Content-Type through to setup_internal_transfer, which applies it after plugin hooks run. This prevents plugins (e.g. xdebug) from overwriting the Content-Type during SEND_RESPONSE_HDR on error pages. Reverts the MIME_PRESENCE_CONTENT_TYPE check in setup_internal_transfer since internal_msg_buffer_type is the correct mechanism for this. Also fixes the x_remap gold file: Content-Type stays text/html (the body is HTML), only X-Original-Content-Type drops the charset suffix.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/proxy/http/HttpTransact.cc:8367

When len == 0, internal_msg_buffer_type is left unchanged. Since free_internal_msg_buffer() does not clear it, this can leave a stale MIME type associated with an empty error body until State::destroy() runs (and potentially returned via TSHttpTxnErrorBodyGet). Consider freeing and nulling s->internal_msg_buffer_type in this branch as well.

  } else {
    s->hdr_info.client_response.field_delete(static_cast<std::string_view>(MIME_FIELD_CONTENT_TYPE));
    s->hdr_info.client_response.field_delete(static_cast<std::string_view>(MIME_FIELD_CONTENT_LANGUAGE));
  }

Copilot · 2026-03-24T18:15:42Z

src/proxy/http/HttpTransact.cc

  if (len > 0) {
    s->hdr_info.client_response.value_set(static_cast<std::string_view>(MIME_FIELD_CONTENT_TYPE), body_type);
    s->hdr_info.client_response.value_set(static_cast<std::string_view>(MIME_FIELD_CONTENT_LANGUAGE), body_language);
+    ats_free(s->internal_msg_buffer_type);
+    s->internal_msg_buffer_type = ats_strdup(body_type);


The PR description mentions fixing HttpSM::setup_internal_transfer to only apply the text/html default when Content-Type is not already present, but setup_internal_transfer() still unconditionally sets Content-Type: text/html when internal_msg_buffer_type is null. If the intent is to preserve a Content-Type set earlier in the response pipeline (e.g., by a plugin), this behavior still overwrites it unless the plugin also populates internal_msg_buffer_type.

Copilot · 2026-03-24T18:15:42Z

doc/admin-guide/monitoring/error-messages.en.rst

+``Content-Charset``
+   The character encoding of the error pages. This value is appended to the ``Content-Type`` header
+   as a ``charset`` parameter. Default: ``utf-8``.
+


The Content-Charset directive is documented as always being appended to the Content-Type header as a charset parameter, but the current implementation only appends ; charset=... when a Content-Type directive is present and does not already include charset=. Consider updating this section to reflect the actual behavior (including what happens when Content-Type is omitted or already has a charset parameter).

Copilot · 2026-03-24T18:15:42Z

tests/gold_tests/body_factory/body_factory_content_type.test.py

+    body factory error responses instead of the hardcoded text/html default.
+
+    Two scenarios:
+    1. Default: no Content-Type directive -> text/html; charset=utf-8


The scenario description in this docstring says the default case returns text/html; charset=utf-8, but the test below only asserts text/html (no charset) for the default path. Please update the docstring to match the behavior being tested.

Suggested change

1. Default: no Content-Type directive -> text/html; charset=utf-8

1. Default: no Content-Type directive -> text/html

Don't silently append "; charset=X" to a custom Content-Type. If the operator writes "Content-Type: text/plain", that's what the client gets. If they want charset, they write "Content-Type: text/plain; charset=utf-8" explicitly.

The error page templates are UTF-8, so the default Content-Type should declare it: "text/html; charset=utf-8". This is more correct per HTTP specs and matches what the body factory was originally producing before setup_internal_transfer was stripping it.

bryancall · 2026-03-24T18:27:24Z

[approve ci autest]

bryancall added this to the 11.0.0 milestone Mar 6, 2026

bryancall self-assigned this Mar 6, 2026

This was referenced Mar 6, 2026

Is there a way to customize body factory response headers? #10893

Closed

☂ Migrate remaining configuration files to YAML format #12753

Open

bryancall requested a review from Copilot March 6, 2026 22:36

bryancall added the Body Factory label Mar 6, 2026

Copilot started reviewing on behalf of bryancall March 6, 2026 22:36 View session

Fix Sphinx docs build: use literal markup for .body_factory_info

1b51b53

The :file: role with nitpicky mode causes an unresolved reference warning for .body_factory_info. Use literal markup instead.

Copilot AI reviewed Mar 6, 2026

View reviewed changes

doc/admin-guide/monitoring/error-messages.en.rst Outdated Show resolved Hide resolved

src/proxy/http/HttpBodyFactory.cc Outdated Show resolved Hide resolved

src/proxy/http/HttpBodyFactory.cc Outdated Show resolved Hide resolved

bryancall requested a review from Copilot March 9, 2026 15:25

Copilot AI reviewed Mar 9, 2026

View reviewed changes

src/proxy/http/HttpBodyFactory.cc Outdated Show resolved Hide resolved

tests/gold_tests/body_factory/body_factory_content_type.test.py Outdated Show resolved Hide resolved

doc/admin-guide/monitoring/error-messages.en.rst Outdated Show resolved Hide resolved

Copilot started reviewing on behalf of bryancall March 9, 2026 16:02 View session

bryancall marked this pull request as draft March 9, 2026 22:18

Omit charset from default Content-Type in body factory responses

92275ad

Only include "; charset=X" when a custom Content-Type is configured in .body_factory_info. Default responses emit just "text/html" to preserve backward-compatible header behavior.

bryancall marked this pull request as ready for review March 16, 2026 16:35

Handle remaining Copilot feedback on Content-Type metadata.

0522902

Avoid duplicating charset when a custom body-factory Content-Type already sets one, normalize Korean language-tag examples to BCP47, and make the autest Content-Type matching more formatting-tolerant.

bryancall added 2 commits March 23, 2026 20:01

bryancall requested a review from Copilot March 24, 2026 18:10

Copilot started reviewing on behalf of bryancall March 24, 2026 18:11 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

bryancall added 2 commits March 24, 2026 11:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Content-Type directive to body factory .body_factory_info#12947

Add Content-Type directive to body factory .body_factory_info#12947
bryancall wants to merge 9 commits intoapache:masterfrom
bryancall:body-factory-content-type

bryancall commented Mar 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bryancall commented Mar 16, 2026

Uh oh!

bryancall commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

bryancall commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	1. Default: no Content-Type directive -> text/html; charset=utf-8
	1. Default: no Content-Type directive -> text/html

Conversation

bryancall commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bryancall commented Mar 16, 2026

Uh oh!

bryancall commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

bryancall commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bryancall commented Mar 6, 2026 •

edited

Loading