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

FullHttpMessageFormatter should be smarter about formatting binary request/response bodies #126

Closed
ostrolucky opened this issue Apr 25, 2020 · 3 comments · Fixed by #128
Closed

Comments

@ostrolucky
Copy link
Contributor

Like most websites, we have some file uploads. We also use Monolog to store logs into Google's Stackdriver. The way how this works is that Google encodes such log record via json_encode. This fails for such requests. Specificially, this is what you get: PHP Warning: InvalidArgumentException: json_encode error: Malformed UTF-8 characters, possibly incorrectly encoded. I can imagine lot of people with different loggers will have problems with your formatters in combination with binary content, not mentioning that it doesn't make much sense to store these messages as binary in traditional log files anyway as well.

What I would suggest is that your FullHttpMessageFormatter detects if content is binary similar as CurlCommandFormatter does and if that's the case, just do bin2hex. I can prepare PR if you are OK with this solution.

@dbu
Copy link
Contributor

dbu commented Apr 27, 2020

yeah, that sounds like a good idea to do. we could also just output a text that says its binary data, but as this is the FullHttpMessageFormatter, bin2hex sounds like a cool solution. i wonder if we then should truncate the bin2hex result, or just output something like -- body is 123456 bytes of binary data -- if the body is longer than the maximum length.

@ostrolucky
Copy link
Contributor Author

That's a good question. But I would say it's a different thing unrelated to binary problem, because right now FullHttpMessageFormatter does just simple cut and doesn't care about informing user in a log that it indeed did some cutting. So we can perhaps take care of that in separate PR.

@dbu
Copy link
Contributor

dbu commented Apr 28, 2020

a fragment of a binary stream is likely less useful than a fragment of json or form-data, but i don't mind either way. doing just the bin2hex and then keep the normal flow would indeed be easiest, we can start with that.

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

Successfully merging a pull request may close this issue.

2 participants