Skip to content

Relax Accept header requirement for JSON-only responses #1500

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Relax Accept header requirement for JSON-only responses #1500

wants to merge 1 commit into from
+39 −8

Conversation

@domdomegg
Copy link
Member

Summary

This PR relaxes the Accept header validation for servers with is_json_response_enabled=True. These servers only return application/json responses and never use SSE, so they should only require application/json in the Accept header rather than requiring both application/json and text/event-stream.

Changes

  • Modified _handle_post_request to conditionally validate the Accept header based on response mode
  • Extracted validation logic into a new _validate_accept_header method to reduce complexity
  • Added test coverage for JSON-only Accept header validation
  • Maintained backward compatibility - requests with both content types still work

Benefits

  • Easier to test JSON-only MCP servers with tools like curl
  • Follows the principle that servers should only require what they actually need to return
  • All existing tests pass

@domdomegg
Relax Accept header requirement for JSON-only responses
f9bf124 
When is_json_response_enabled is True, servers only return application/json
responses and never use SSE. This change relaxes the Accept header validation
to only require application/json in this mode, rather than requiring both
application/json and text/event-stream.

This makes it easier to test JSON-only MCP servers with tools like curl,
which is useful when developing and debugging MCP servers.

For servers with is_json_response_enabled=False (SSE mode), the existing
requirement for both content types is maintained.
@domdomegg domdomegg requested a review from maxisbey October 20, 2025 19:06
felixweinberger
felixweinberger requested changes Oct 23, 2025
View reviewed changes
Copy link
Contributor

@felixweinberger felixweinberger left a comment

Choose a reason for hiding this comment

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

Should we add tests that also cover the failure cases of this new validation?

E.g. missing Accept header and incorrect accept header (e.g. sending event/stream on a JSON server)

I think these should return 406

@felixweinberger felixweinberger added enhancement New feature or request needs more work Not ready to be merged yet, needs additional changes. labels Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixweinberger felixweinberger felixweinberger requested changes

@maxisbey maxisbey Awaiting requested review from maxisbey

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

enhancement New feature or request needs more work Not ready to be merged yet, needs additional changes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@domdomegg @felixweinberger