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

enforce content type on XML-RPC. addresses #3991 #3993

Merged
merged 2 commits into from Aug 21, 2023
Merged

Conversation

splitbrain
Copy link
Collaborator

This ensures only text/xml or application/xml content types are accepted when communicating with the XML-RPC API

This ensures only text/xml or application/xml content types are accepted
when communicating with the XML-RPC API
Copy link
Collaborator

@michitux michitux left a comment

Choose a reason for hiding this comment

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

Looks good to me. I would have added one or two test cases to check that the exception is actually thrown when the content type is not set/has the wrong value but that's a detail. Also, content types are case insensitive. I don't know if PHP does any normalization, but to be fully correct it could make sense to match case insensitively and also to ignore parameters (text following after ;) - this could be a character set, no idea if clients set that.

Do you have any idea about the impact on existing users, i.e., do the existing XML-RPC client implementations actually send a correct content type? According to the specification, the content type needs to be text/xml so requiring it seems to be following the specification but I wouldn't be surprised if there were clients not following the specification.

An alternative if this should break many/important/difficult to fix XML-RPC clients would be to instead check that the mime type is not one of those mime types that are allowed in "simple" requests (in particular, according to my understanding, it should be safe to allow requests without content type header). I've implemented such a check in XWiki's REST API recently. Note that the mime type can also contain a character set (in particular for text/plain) and is case-insensitive, so doing a case-insensitive prefix match seemed to be right choice. In XWiki, we now require a CSRF token in these cases as XWiki explicitly supports some of these content types.

As I've said, the situation should be much simpler here. I'm just wondering if existing XML-RPC clients actually send the content type header.

@splitbrain splitbrain merged commit e3029c0 into master Aug 21, 2023
16 checks passed
@splitbrain splitbrain deleted the xmlrpcfix branch August 21, 2023 16:02
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.

None yet

2 participants