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.

Sign up for GitHub

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

Jump to bottom

Hoverfly Spy mode and nginx proxy_pass do not work together #944

Closed
ttiurani opened this issue Aug 25, 2020 · 9 comments
Closed

Hoverfly Spy mode and nginx proxy_pass do not work together #944

ttiurani opened this issue Aug 25, 2020 · 9 comments
Labels
bug

Comments

@ttiurani
Copy link
Contributor

ttiurani commented Aug 25, 2020 •
edited
Loading

The repository:

https://github.com/ttiurani/hoverflybug

contains instructions on how to reproduce with a local docker the bug with nginx's default proxy_pass not working with Hoverfly spy mode, when doing a PUT/POST with a body and there being a "headers" request matcher in Hoverfly's simulation file. Nginx fails the request Hoverfly sends with 400, where without hoverfly as proxy – or without any "headers" request matchers – things work.

Replicates with the latest Hoverfly.
@tommysitu tommysitu added the bug label Aug 25, 2020
@tommysitu
Copy link
Member

It's a very weird edge case. It doesn't work in spy mode with a header matcher in the simulation for certain condition:

✅ spy mode with header matcher; the request is matched: Got 200 with simulated response
❌ spy mode with header matcher; the request is not matched: Got 400 from upstream nginx
✅ spy mode without header matcher; the request is matched: Got 200 with simulated response
✅ spy mode without header matcher; the request is not matched: Got the response from real server

When it fails, I got something like this:

< HTTP/1.1 400 Bad Request
< Server: nginx/1.17.4
< Date: Tue, 25 Aug 2020 21:56:20 GMT
< Content-Type: text/html
< Content-Length: 155
< Connection: keep-alive
< Cf-Ray: -
< Hoverfly: Was-Here
< Hoverfly: Forwarded
<
<html>
<head><title>400 Bad Request</title></head>
<body>
<center><h1>400 Bad Request</h1></center>
<hr><center>cloudflare</center>
</body>
</html>

Is it similar to what you get as well @ttiurani ?

I'm not sure what the cause is yet, but will try to debug

@ttiurani
Copy link
Contributor Author

ttiurani commented Aug 26, 2020 •
edited
Loading

Is it similar to what you get as well @ttiurani ?

Yes, exactly the issue. Furthermore, if the request doesn't have a body at all, the passthrough seems to also work even with headers matchers. It's unfortunate nginx doesn't tell what exactly is wrong with it, but I'm thinking, because it needs the body, Content-Length might be the culprit here, and this would be somehow related to #818 , could it @tommysitu ?

@ttiurani
Copy link
Contributor Author

Noticed a very likely related bug in templating:

If I have a "headers" matcher in my simulation, e.g.:

          "headers": {
            "Authorization": [
              {
                "matcher": "glob",
                "value": "Bearer test-*"
              }
            ]
          }

then in the response body a {{ Request.Header.Authorization }} returns an empty string. But if I remove the request "headers" matcher, I get the correct value printed to the response. @tommysitu

tommysitu added a commit that referenced this issue Aug 30, 2020
@tommysitu
#944 clone headers when copying request details, forward original req…
b66e35e 
…uest in spy mode
@tommysitu
Copy link
Member

tommysitu commented Aug 30, 2020 •
edited
Loading

Hey @ttiurani I spotted where the problem is, the Hoverfly matching logic converts the header names to lower case, which somehow upset Nginx. Cloning the headers before matching should fix it: #945

Also removing the bit that reconstructs request, because we can just forward the original request if no matching simulation found

tommysitu added a commit that referenced this issue Aug 30, 2020
@tommysitu
#944 clone headers when copying request details, forward original req…
60b4aa2 
…uest in spy mode
@tommysitu tommysitu mentioned this issue Aug 30, 2020
Closed
@tommysitu
Copy link
Member

I've created another issue to keep track of the related bug

@ttiurani
Copy link
Contributor Author

Thanks for the fixes! However, I did unfortunately now bump into a new bug:

INFO[2020-08-31T16:12:45Z] Going to call real server
ERRO[2020-08-31T16:12:45Z] There was an error when forwarding the request to the intended destination  error="Post \"/login" unsupported protocol scheme \"\"" mode=spy request="&map[body:
...
method:POST path:/login query:map[so:[00D4E0000000mMM]] scheme:http]" response="&map[error:nil response]

and nothing actually gets sent to nginx. I did build this myself, so first I thought this was about me somehow building the linux/amd64 binary wrong, but then I tried to build the commit 9a0eee5 and did manage to get the same 400 error from nginx just fine.

So did your "Also removing the bit that reconstructs request" part perhaps now lose the original scheme? @tommysitu

@tommysitu
Copy link
Member

Oh no, what request did you make?

@ttiurani
Copy link
Contributor Author

ttiurani commented Aug 31, 2020 •
edited
Loading

It's the original involved code I used to find this bug, the body of a POST is big in this one, it is -plain-http-tunneling and also Netty, but the request looks fine. It even has in that journal log entry end the scheme:http.

I didn't yet try without -plain-http-tunneling and curl if it replicates or not.

@ttiurani ttiurani mentioned this issue Sep 1, 2020
Closed
@ttiurani
Copy link
Contributor Author

ttiurani commented Sep 1, 2020 •
edited
Loading

I created a new issue about this to #948 – without -plain-http-tunneling the 400 bug is indeed fixed now, which is great, but with -plain-http-tunneling curl just hangs when trying to call real server @tommysitu

tommysitu added a commit that referenced this issue May 8, 2021
@tommysitu
#944 clone headers when copying request details, forward original req…
d213537 
…uest in spy mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ttiurani @tommysitu