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

Warning on content length always displayed on import. #763

Closed
dpm3354 opened this issue Aug 21, 2018 · 7 comments
Closed

Warning on content length always displayed on import. #763

dpm3354 opened this issue Aug 21, 2018 · 7 comments
Assignees
Labels

Comments

@dpm3354
Copy link

dpm3354 commented Aug 21, 2018

I see the fix for the content length I mentioned, but there appears to be a problem with it. I’m guessing someone forgot to test this….

Case #1 - Change the simulation.json file after export and before import. Make sure the number of characters in the body DOES NOT CHANGE. For instance, I changed one ascii character in the body to a different ascii character, but the length of the body did not change.

552 hoverctl start
553 hoverctl stop
554 hoverctl start
555 hoverctl logs
556 hoverctl mode capture
557 hoverctl logs
558 hoverctl export simulation.json
559 hoverctl stop
560 hoverctl start
561 hoverctl import simulation.json (edited)
That’s my history.
between #558 and #561 I did NOT change the simulation.json file.
But I still get this warning after the cmd at line 561: “WARNING: Response contains incorrect Content-Length header on data.pairs[1].response, please correct or remove header”

If the content didn’t change, the warning should not appear.

Case #2 - Change nothing in the simulation.json file after export and before import.

Notice the warning for content length is echoed twice to the console.

@JohnFDavenport
Copy link
Contributor

JohnFDavenport commented Aug 21, 2018

Can you post the 2 simulation.json files @dpm3354 ? Or send them privately?

@dpm3354
Copy link
Author

dpm3354 commented Aug 21, 2018

Sorry, John. I can't put more time into this right now. And I already deleted the files. Did you try on your own with your own file?

@JohnFDavenport
Copy link
Contributor

Paul, we have not managed to reproduce a problem. That is, as long as we preserve the content length correctly the warning does not appear. As soon as we add or subtract characters we see the warning.

We decided that Hoverfly should not automatically adjust the Content-Length as it may be the intent to simulate an incorrect correct-length and it would be more confusing to second-guess intentions. It's also very complicated to calculate the content length when the user has used templating.

As the warning says you can adjust the content-length or remove the header, in which case Hoverfly will default to use Transfer-Encoding chunked to read the body.

Can you try again. If you see the same problem please supply us with the 'before' and 'after' JSON files.

@john4744
Copy link
Contributor

I'm seeing this warning too, I think it might happen when the body of the response is encoded. During this check https://github.com/SpectoLabs/hoverfly/blob/master/core/import.go#L162 maybe pairView.Response.Body is still base64 encoded, and the Content-Length header is reporting the decoded size?

@JohnFDavenport
Copy link
Contributor

Thanks @john4744 - we'll have a look at that.

@benjih
Copy link
Contributor

benjih commented Sep 6, 2018

Hey @john4744,

What a great spot. When I was testing this, I was trying with JSON apis, so Hoverfly would not encode the response bodies. I've just tried this with an image (guaranteed to get encoded by Hoverfly) and was able to recreate the problem.

I have attached a simulation with an encoded body that triggered the error for me with Hoverfly v0.17.3.

dog.txt

I will look into fixing this now.

@JohnFDavenport
Copy link
Contributor

The problem reported by @john4744 has been fixed in v0.1.7.5

We can't tell if we have fixed the original issue but given we haven't had any further info I'm closing this issue. Feel free to reopen if there is a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants