Allow to disable body limitation size

[jdecool]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Related tickets	-
Documentation	php-http/documentation#259
License	MIT

What's in this PR?

This PR allow to set the httpplug.profiling.captured_body_length to NULL to avoid body truncate in the Symfony profiler.

Why?

I want to view all the body content of a Request or Response.

Checklist

• Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
• Documentation pull request created (if not simply a bugfix)

To Do

• If accepted, update documentation

[joelwurtz]

Thanks, LGTM,

[jdecool]

I've created the documentation PR, and update this PR description.

[dbu]

i am not sure if this is a good idea. you could set the limit to 100k or whatever crazy high value. removing the limit could break in bad ways when you ever happen to load several megabyte of data. then again, the way FullHttpMessageFormatter is implemented, the whole body is converted to string anyways.

i am +0 on this. if the other maintainers like it i won't oppose merging it. however, if we want to do this, we have to adjust the constructor of FullHttpMessageFormatter so that this not only works accidentally but is explicit.

when we move php-http/message to PHP 7.1+, we will do strict type declarations and atm, the $maxBodyLength would then be forced to int. you'd need to adjust the phpdoc on the parameter and on the property to int|null. we should also adjust addBody to handle null explicitly, even though it accidentally works because mb_substr($string, 0, null) takes the whole string.

[jdecool]

i am not sure if this is a good idea. you could set the limit to 100k or whatever crazy high value. removing the limit could break in bad ways when you ever happen to load several megabyte of data.

Sure, it could be dangerous. But when I use the Symfony profiler (in dev mode), I don't want that my body content will be truncate.

In a debug purpose, I want to have all information I need.

I'm going to open a new PR in the message component to update the formatter.

[dbu]

I'm going to open a new PR in the message component to update the formatter.

i just changed the code that you will adjust, please check the newest master: php-http/message#111

[jdecool]

i just changed the code that you will adjust, please check the newest master: php-http/message#111

👍

[joelwurtz]

@dbu I think there is some cases where it's useful, specially when uou know that the remote api will never throw a too big response.

As an example when using a client for elasticsearch, i know that i will never get a too big output, and that i can always show the the full body, (but i often hit the default limit here).

[dbu]

apart from that i am okay to merge this as soon as we have the merge request against php-http/message to state that null is explicitly supported.

[dbu]

thanks for this contribution (and your patience while we figured out how to exactly do this ;-) )

[jdecool]

No problem 😄

[xabbuh]

Sorry for being late, but wouldn't it be more straight-forward to use something like ifTrue(...)->thenInvalid() construct?

What I have in mind would be something like this:

->scalarNode('captured_body_length')
    ->validate()
        ->ifTrue(function ($v) {
            return null !== $v && !is_int($v);
        })
        ->thenInvalid('The child node "captured_body_length" at path "httplug.profiling" must be an integer or null ("%s" given).')
    ->end()
->end()

[dbu]

agreed. i created #323 with your proposed change