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

Handle CONTENT_TYPE header containing charset correctly #51

Merged

Conversation

Flixt
Copy link
Contributor

@Flixt Flixt commented Mar 17, 2023

We encountered a problem using Microsoft Azure AD and traced it down to Microsoft sending application/scim+json; charset=utf-8 in the CONTENT_TYPE header when calling POST /Users.

Since the CONTENT_TYPE header can contain more than just the Mime-Type scimitar should use start_with? instead of ==. This PR changes that.

PS: I'm not sure if stripping the charset away from the CONTENT_TPYE header is wise. I just added that because the tests looked "more correct" to me first sight. If you know more or have objections just drop me a line.

@pond
Copy link
Member

pond commented Mar 19, 2023

Thanks for the PR. This is definitely an omission/bug in Scimitar.

I note from https://www.rubydoc.info/github/rack/rack/Rack%2FRequest%2FHelpers:media_type that we should probably be using media_type not content_type here, so that the charset part does not need to be otherwise stripped or ignored. Would you be willing to give that a try as a new commit in this PR, or prefer to keep this submission as-is?

@Flixt Flixt force-pushed the support-content-type-with-charset branch from 2518551 to 483e0f2 Compare March 20, 2023 07:56
@Flixt
Copy link
Contributor Author

Flixt commented Mar 20, 2023

Good catch, media_type seems to be the way to go. I changed the commit in this PR to use media_type and leave the CONTENT_TYPE header as it was.

@pond
Copy link
Member

pond commented Mar 20, 2023

That's great, thanks! I'll release that as 2.4.2 shortly.

@pond pond merged commit 9d4b348 into RIPAGlobal:main Mar 20, 2023
This was referenced Mar 20, 2023
@pond
Copy link
Member

pond commented Mar 20, 2023

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