[RFC] throwable_string_param_max_len: Configurable string length in getTraceAsString()

Posted 4 months ago by tyson andre — view source

unread

Hi internals,

I've created a new RFC https://wiki.php.net/rfc/throwable_string_param_max_len

Since 2003, Throwable->getTraceAsString() and Throwable->__toString()
have limited the length of string function arguments in stringified stack traces to 15 bytes
(e.g. #0 /path/to/file.php(line) function(“012345678901234…”).
This is not enough space to render information such as paths, URLs, UUIDs, etc.
if an end user wants to see them when debugging an issue or developing locally.

This proposes a new ini setting throwable_string_param_max_len that would allow changing the string byte limit to any value between 15 and 1000000, keeping the current default of 15 bytes.

Earlier discussion can be found in https://externals.io/message/110717
(Making the hardcoded string length limit of Throwable->getTraceAsString() configurable)

Thanks,

Tyson

Posted 4 months ago by Dan Ackroyd — view source

unread

I've created a new RFC https://wiki.php.net/rfc/throwable_string_param_max_len

How come you're proposing an ini setting instead of adding a parameter
to getTraceAsString() that specifies the length for params?

Also, I didn't understand why there was a problem with formatting
traces in userland. I saw a link to some code, but no clear
description of what the problem was.

cheers
Dan
Ack

Posted 4 months ago by tyson andre — view source

unread

I've created a new RFC https://wiki.php.net/rfc/throwable_string_param_max_len

How come you're proposing an ini setting instead of adding a parameter
to getTraceAsString() that specifies the length for params?

I don't expect many people to use a parameter for getTraceAsString(),
and that alternative doesn't help with implicit __toString() or the default output for fatal throwables.

The application developer and end user may have different ideas of what to use as a length.
If an application would manually use getTraceAsString(Config::getMaxStringLengthForEnvironment()),
it'd be much shorter as an ini setting.

Also, I didn't understand why there was a problem with formatting
traces in userland. I saw a link to some code, but no clear
description of what the problem was.

If there's an uncaught exception and no exception handler,
you'll get it truncated to 15 bytes.
Setting up a more complicated solution to format traces of uncaught traces in userland is probably not worth the effort for short php scripts,
and many people would call getTraceAsString() over getTrace() for the convenience (especially in rarely used code paths).

If you're an end user of an application or composer library,
patching the code to format traces in userland is inconvenient if the application developers are using stringified exceptions or getTraceAsString().

Tyson

Posted 4 months ago by tyson andre — view source

unread

Hi Dan Ack,

Also, I didn't understand why there was a problem with formatting
traces in userland. I saw a link to some code, but no clear
description of what the problem was.

I expanded the description of how getTraceAsString() might be improperly used in existing code and moved it to
https://wiki.php.net/rfc/throwable_string_param_max_len#impact_of_raising_string_param_length_limit
(and how the code would already be unsafe, but raising the limit may make the impact of unsafe code like that worse)

(e.g. if the code or dependency may echo $exception if it was written by an author unaware of potential https://en.wikipedia.org/wiki/Cross-site_scripting or sensitive data exposure)

This is mostly included to explain why I don't propose raising the defaults in the RFC

Tyson

Posted 4 months ago by tyson andre — view source

unread

Hi internals,

I plan to start the voting for https://wiki.php.net/rfc/throwable_string_param_max_len on July 11, 2020.

Since 2003, Throwable->getTraceAsString() and Throwable->__toString()
have limited the length of string function arguments in stringified stack traces to 15 bytes
(e.g. #0 /path/to/file.php(line) function("012345678901234…").
This is not enough space to render information such as paths, URLs, UUIDs, etc.
if an end user wants to see them when debugging an issue or developing locally.

This proposes a new ini setting throwable_string_param_max_len that would allow
changing the string byte limit to any value between 15 and 1000000, keeping the current default of 15 bytes.

Thanks,

Tyson

Posted 4 months ago by Nikita Popov — view source

unread

On Wed, Jul 1, 2020 at 8:55 PM tyson andre [email protected]
wrote:

Hi internals,

I plan to start the voting for
https://wiki.php.net/rfc/throwable_string_param_max_len on July 11, 2020.

Why is the lower limit of the ini setting 15 and not 0?

Nikita

Posted 4 months ago by Nikita Popov — view source

unread

On Wed, Jul 1, 2020 at 8:55 PM tyson andre [email protected]
wrote:

Hi internals,

I plan to start the voting for
https://wiki.php.net/rfc/throwable_string_param_max_len on July 11, 2020.

Why is the lower limit of the ini setting 15 and not 0?

I just saw that you mention this possibility as "future scope". I don't
think your reasoning there holds up if you take into account that we
already introduced zend.exception_ignore_args in PHP 7.4 -- and which is
enabled by default in production. That removes argument information from
exceptions completely. Setting the string parameter length to 0 still
provides more information than that.

Regards,
Nikita

Posted 4 months ago by tyson andre — view source

unread

Hi Nikita,

Why is the lower limit of the ini setting 15 and not 0?

I just saw that you mention this possibility as "future scope".
I don't think your reasoning there holds up if you take into account
that we already introduced zend.exception_ignore_args in PHP 7.4 --
and which is enabled by default in production.
That removes argument information from exceptions completely.
Setting the string parameter length to 0 still provides more information than that.

I wasn't very familiar with what zend.exception_ignore_args did, so I missed that.
You do have a good point - it may be useful in use cases where string contents should be completely hidden
but it's still useful to log arg types.

I updated https://wiki.php.net/rfc/throwable_string_param_max_len - the minimum is now 0,
and the suggested value in php.ini-production is now 0, and the default remains at 15.

phpdoc-en-git ±master » ag exception_ignore_args
appendices/migration74/other-changes.xml
34:   <literal>zend.exception_ignore_args</literal> is a new INI directive

Aside: I think that should be documented in https://www.php.net/manual/en/errorfunc.configuration.php or somewhere else.
Right now, the documentation isn't on php.net, just the changelog.

Thanks,

Tyson

Posted 4 months ago by Nikita Popov — view source

unread

On Wed, Jul 1, 2020 at 10:46 PM tyson andre [email protected]
wrote:

Hi Nikita,

Why is the lower limit of the ini setting 15 and not 0?

I just saw that you mention this possibility as "future scope".
I don't think your reasoning there holds up if you take into account
that we already introduced zend.exception_ignore_args in PHP 7.4 --
and which is enabled by default in production.
That removes argument information from exceptions completely.
Setting the string parameter length to 0 still provides more information
than that.

I wasn't very familiar with what zend.exception_ignore_args did, so I
missed that.
You do have a good point - it may be useful in use cases where string
contents should be completely hidden
but it's still useful to log arg types.

I updated https://wiki.php.net/rfc/throwable_string_param_max_len - the
minimum is now 0,
and the suggested value in php.ini-production is now 0, and the default
remains at 15.
phpdoc-en-git ±master » ag exception_ignore_args
appendices/migration74/other-changes.xml
34:   <literal>zend.exception_ignore_args</literal> is a new INI directive
Aside: I think that should be documented in
https://www.php.net/manual/en/errorfunc.configuration.php or somewhere
else.
Right now, the documentation isn't on php.net, just the changelog.

Thanks for the update. You'll probably want to remove the corresponding
section under future scope now.

You may also want to give some more thought about the name of the ini
setting name. We already have zend.exception_ignore_args (which is TBH not
a great name), and now we get throwable_string_param_max_len, which doesn't
follow the same format. Possibly zend.exception_string_param_max_len would
fit in better?

Nikita

Posted 4 months ago by tyson andre — view source

unread

Hi Nikita,

Thanks for the update. You'll probably want to remove the corresponding section under future scope now.

Fixed, forgot to delete that after copying parts of it.

You may also want to give some more thought about the name of the ini setting name.
We already have zend.exception_ignore_args (which is TBH not a great name),
and now we get throwable_string_param_max_len, which doesn't follow the same format.
Possibly zend.exception_string_param_max_len would fit in better?

I'd agree with zend.exception_ignore_args not being a great name, but maintaining the same prefix would make it easier to find/remember correctly.
I was considering asking about what people thought about zend.exception_string_param_max_len.

I've updated https://wiki.php.net/rfc/throwable_string_param_max_len

Thanks,

Tyson