Closed brettneese closed 7 years ago
Thanks for the report, this is very detailed and I appreciate the research you've done!
Indeed I do hope to have that headers issue resolved by the next release (it's not a difficult change, just waiting to hear back on how things are going), which may help with this issue.
What happens if you add another proxy_header line:
proxy_header Content-Type application/json
Does that help?
No, I don't have the logs right now but when we did that we got a null pointer exception from within the Jetbrains stuff (it's very picky.)
We're also running into some issues that seem to be related to headers with regards to websockets. Any update on when the header issue will be fixed?
We're also running into some issues that seem to be related to headers with regards to websockets.
What kinds of issues?
What kinds of issues?
@molt maybe this issue is related to #713? -- both use Jetty
Yeah, that's very likely it. We thought it might be related to the websocket directive only applying headers before the request, or something, but it's a Jetty-powered app so #713 seems plausible. (I actually don't have access to that box or the logs right now, sorry if I'm being vague.)
Brett Neese 563-210-3459
On Mon, Apr 11, 2016 at 3:48 PM, elcore notifications@github.com wrote:
What kinds of issues?
@molt https://github.com/molt maybe this issue is related to #713 https://github.com/mholt/caddy/pull/713?
— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/mholt/caddy/issues/719#issuecomment-208555491
Okay, so that is probably fixed. I want to be sure we understand the content-type problem fully. So the proxy setup looks like this:
[==========request==========] [======response======]
client ---> caddy ---> jetty ---> caddy ---> client
A B C D
So you're suggesting, that at A
the Content-Type is application/json
but was changed to text/plain
at B
? Or is it something else.
Actually, based on what I was seeing, it seemed like the request pipeline was just fine, but the content type got changed to something unexpected ("text/plain") between C and D.
So the client in this case is JetBrains teamware, and the backend is some jetty instance.
Okay, this is helpful. Someone (if not me) will look into this soon. If you want to get a head start, I'd recommend looking in here and maybe adding some print statements to verify that the Content-Type header is being copied to downstream properly.
Right. Well, more specifically, the client is one JetBrains application (e.g. YouTrack), and the backend is the API of another JetBrains application (Hub, in this case).
Wonder if the changed header value can be reproduced without needing JetBrains.
If you want, the teamware is available for free download, though it's kind of a pain to install: https://www.jetbrains.com/youtrack/, https://www.jetbrains.com/hub/. We're running on a Centos 7 box.
We ended up using a Elastic Load Balancer to do the proxying in Hub for now, though that's a hacky solution and definitely overkill.
At this point, however, we kind of just wanted to wait to see what happens when #666 gets first before trying anything else.
Brett Neese 563-210-3459
On Sat, Apr 16, 2016 at 12:44 PM, Matt Holt notifications@github.com wrote:
Wonder if the changed header value can be reproduced without needing JetBrains.
— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/mholt/caddy/issues/719#issuecomment-210862549
@brettneese Thanks. #666 has been implemented now, ready to give it another try?
The same error in version 0.9.
@mholt We ended up throwing everything on Dokku, for this and other reasons. If I don't get too bogged down I can try things anything sometime this week but it's looking to be v busy.
Running head on into this issue now. Getting the
MessageBodyReader not found for media type=text/plain;charset=utf-8, type=class jetbrains.jetpass.rest.dto.ServiceJSON, genericType=class jetbrains.jetpass.rest.dto.ServiceJSON.
Error with just about any configuration I can come up with. Using an application/json upstream or downstream ends up sending the actual users json but the YouTrack application still throws a fit about not getting JSON back.
More than happy to give access to some test environments, as YouTrack and Hub are both Composed and are free for 10 user accounts or less.
Is this still an issue with the latest version, 0.9.5?
I've had to abandon the JetBrains software for the time being, too many other higher priorities. I'd be happy to open a new ticket with full debug information, instead of just a "Me too!" when I get back to trying the teamware software if you'd like to close out this ticket.
@dschaper Sure, if you find that the problem still exists, feel free to post it here.
I confirm the issue is still there, I was trying to setup Jetbrains softwares behind Caddy and I'm getting the same error.
Fortunately, I reproduce this issue on my local and did some investigation.
Yes, this is indeed a bug, the unexpected header Content-Type=text/plain;charset=utf-8
caused YouTrack
crashed. This header isn't set by Caddy
nor Hub
backend, but go std lib.
I have just captured the culprit packet with the help of wireshark
.
Here is the original packet sending from Hub
to Caddy
:
Hypertext Transfer Protocol
HTTP/1.1 200 OK\r\n
Date: Wed, 21 Jun 2017 00:23:46 GMT\r\n
Cache-Control: no-cache, no-store, no-transform, must-revalidate\r\n
Content-Length: 0\r\n
Server: Jetty(9.2.10.v20150310)\r\n
\r\n
And this is packet which Caddy
proxy to client (YouTrack
in my case):
Hypertext Transfer Protocol
HTTP/1.1 200 OK\r\n
Cache-Control: no-cache, no-store, no-transform, must-revalidate\r\n
Content-Length: 0\r\n
Date: Wed, 21 Jun 2017 00:23:46 GMT\r\n
Server: Jetty(9.2.10.v20150310)\r\n
Content-Type: text/plain; charset=utf-8\r\n
\r\n
Obviously, A Content-Type
has been added even there is no body (Conent-Length is 0).
In most scenarios, it isn't harmful, but Jetbrains
software seems concern about it particularly.
After I have forbidden the behavior which go std lib does, the issue gone.
However, I don't think we should add some tricks in Caddy
to workaroud this special issue, it's better to fix in Jetbrains
software instead. It should skip the Content-Type
if there is no body actually.
Tw, you're amazing. Thanks so much for investigating this!!
It does seem strange that we add (or Go adds) a Content-Type header when there is no body. I agree with Tw that this is a bug in Jetbrains and that they should fix their software, but I'd be interested to know your patch, Tw, for getting rid of the Content-Type header. I think it's worth seeing what it would take to fix this in Caddy so we don't add headers that aren't needed.
@mholt This is my scary test patch:
diff --git a/caddyhttp/proxy/reverseproxy.go b/caddyhttp/proxy/reverseproxy.go
index 5fc4aed..465e4a2 100644
--- a/caddyhttp/proxy/reverseproxy.go
+++ b/caddyhttp/proxy/reverseproxy.go
@@ -420,6 +420,14 @@ func copyHeader(dst, src http.Header) {
dst.Add(k, v)
}
}
+
+ _, hasSetCT := dst["Content-Type"]
+ _, hasCT := src["Content-Type"]
+ cl := dst.Get("Content-Length")
+ if !hasSetCT && !hasCT && cl == "0" {
+ // dst.Set("Content-Type", "") // blank Content-Type also trigger the crash
+ dst.Set("Content-Type", "application/json") // add it anyway for test
+ }
}
This will prevent Go from adding the Content-Type because we set it explicitly. I have also try to prevent Go from detecting the Content-Type, it also works as expected. Here is the patch:
diff --git a/src/net/http/server.go b/src/net/http/server.go
index add05c2..a48b88b 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -1304,7 +1304,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
if bodyAllowedForStatus(code) {
// If no content type, apply sniffing algorithm to body.
_, haveType := header["Content-Type"]
- if !haveType && !hasTE {
+ if !haveType && !hasTE && len(p) > 0 {
setHeader.contentType = DetectContentType(p)
}
} else {
Interesting, thanks for sharing that. Don't you think it is a bug in Go if Go is adding a Content-Type when the Content-Length is 0?
Hmm, I think so before, but it seems to be expected after I have seen this test case. And it also caused a lot of other test failures.
@mholt I have just opened a ticket on Go anyway.
@tw4452852 Awesome! That's great, it looks like your fix will go in Go 1.10, which should resolve this issue. As far as I can tell, nothing will need to change within Caddy, is that right?
Yes.
Gonna clear up our TODO list by closing this issue, since it seems the best and right place to fix this is in Go itself. :) Thanks again so much for looking into this, @tw4452852 !
Tagging as deferred since it's out of this repo's control (we revisit deferred issues even though they're closed so we can check up on their progress, see if someone implemented a patch, etc).
Confirming - building Caddy with go1.10beta2 fixes this problem with JetBrains products.
We are trying to run JetBrains Teamware, a set of Java apps that have their own webservers (Jetty, it seems), through a Caddy reverse proxy. We're having an issue, however, with the apps crashing under Caddy when they talk to JetBrains' authentication service, which we think has something to do with the
content-type
header.1. What version of Caddy are you running (
caddy -version
)?Caddy 0.8.2
2. What are you trying to do?
Basically, each of the apps (a CD service, a code review service, etc) talks to a JetBrains "Hub," or authentication service, that operates with oAuth and a few other things. Each of the apps has an internal "Hub" but in order to tie them together you need to be running an external instance of a hub. That's fine, but when they boot up and try and connect to the Hub under Caddy, we encounter the issue specified here: https://youtrack.jetbrains.com/oauth?state=%2Fissue%2FJPS-3181 - with exactly the same error output.
We think the problem may be outlined in this comment:
But through our poking around doesn't actually appear that the content-type on that request is getting changed/added via Caddy -- and nothing in the Caddy code looks to us like the content-types would be getting replaced.
Here's our access log for those requests:
But we do not have a problem running the app without Caddy, so it must be an issue with Caddy. Our initial thought was to overwrite the headers anyway, but we can't manually specify mimetypes due to #666 -- they'd simply get appended, rather than replaced.
Curiously, according to the docs:
but again, we don't see where in Caddy the Content-Type would be getting inferred/set in the first place. Where in the Caddy code would that automatic detection be taking place?
The Hub docs suggest this as the setup for an Nginx/Apache proxy, and we think we emulated that with Caddy, except for the part about 'DefaultType,' directive which Caddy does not have. (And if I knew how the content-type headers were getting coerced, I'd just build one.)
3. What is your entire Caddyfile?
4. How did you run Caddy (give the full command and describe the execution environment)?
We're running it via a systemd service, in Centos 6, under a non-root
centos
user.5. What did you expect to see?
We expect the JetBrains teamware not to crash. ;-)
6. What did you see instead (give full error messages and/or log)?
The error linked above. Specifically, on our machine (sorry for the fugly Java stacktrace):
The full debug output is here.
Any suggestions? We're also going to submit this issue to JetBrains, but we're pretty sure it has something to do with Caddy because simply accessing the Hub on its port works just fine.
(Tagging my colleague, @mnaughto, who's helping me with this.)