Problem/Motivation
Tests that rely on the behavior of ContentTypeHeaderMatcher's content type check can fail in some nginx configurations. I'm marking this as minor because I don't think we really "support" running tests on nginx but at the same time this test failure exposes some inconsistent behaviors for rest resources when running on nginx where the improved DX from #2811133: Confusing error response set by ContentTypeHeaderMatcher when forgetting the Content-Type request header doesn't work as expected.
The problem comes from the fact that when using fast cgi configurations based on nginx example you will end up alwayspassing a content type to php regardless of whether the client supplied it. This causes the has check to fail say "yeah there's a header!" even though there really isn't.
nginx fastcgi reference: https://www.nginx.com/resources/wiki/start/topics/examples/full/#fastcgi...
Proposed resolution
Convert the check from has to a truthy test on the value. "" should is functionally the same as not existing so this should be fine.
Remaining tasks
- Patch
- Review
User interface changes
n/a
API changes
n/a
Data model changes
n/a
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#2 | 3090626-1.patch | 792 bytes | neclimdul |
Comments
Comment #2
neclimdulLets see if this breaks anything.
Comment #3
Wim LeersMakes sense, thank you!
Comment #4
alexpottThis is the only instance of
headers->has('Content-Type')
in core.Committed and pushed f995409be4 to 9.0.x and cb944b89bb to 8.9.x and 7dee9da351 to 8.8.x. Thanks!
Backported to 8.8.x as a test-only change.
Comment #8
mr.baileysRelated: #2860913: Use "missing content-type" exception message for empty content-type header.
Comment #10
Wim Leers#2860913: Use "missing content-type" exception message for empty content-type header. was not just a related issue, it was a duplicate issue that even predates this one.
Crediting people who worked on the duplicate issue.
Comment #11
neclimdulYeah, sorry. I searched but didn't get the right keywords I guess and failed to find it :(
Comment #12
Wim LeersNo worries, @neclimdul :)