proxy: Split out exception for missing content types #32
Conversation
This takes the current logic for checking whether the request is for a valid HTTP content-type and splits it into two parts. The split moves to allow differentiation between a content-type that we don't intend to proxy and a missing content-type as they should be handled differently.
|
Looks like at least one of these failures is also happening on master. |
| // check content type | ||
| contentType := resp.Header.Get("Content-Type") | ||
|
|
||
| if contentType == "" { |
There was a problem hiding this comment.
Am I correct in my assumption that this is an optimization?
The regex match /should/ fail (non-match) for empty content types, but not having to do the regex in cases where content-type is empty seems like a valid optimization case for sure. Just wanted to make sure the reasoning matched my expectation.
There was a problem hiding this comment.
Not an optimisation but ability to differentiate an unsupported content type and a missing one. These are two different scenarios and while you could lump them together, you probably want to have insight into each case instead of treating both the same way.
Apologies for not being clear enough about that in my description.
There was a problem hiding this comment.
Ah. That makes sense. Treating them differently here isn't really leaking anything of value either. 👍
|
The tests currently rely on a few public servers for a couple of tests. A few of those tests sometimes fail, when the server it tests against breaks or has errors. I'll look into this one in particular and see if the test needs to be adjusted. |
|
lgtm. Thanks! |
This takes the current logic for checking whether the request is for a
valid HTTP content-type and splits it into two parts. The split moves to
allow differentiation between a content-type that we don't intend to
proxy and a missing content-type as they should be handled differently.