Everything

php

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

php.internals

tyson andre

6 years ago

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

Dan Ackroyd

6 years ago

On Sat, 27 Jun 2020 at 16:16, tyson andre <tysonandre775@hotmail.com> wrote:

>
> 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

tyson andre

6 years ago

tyson andre

6 years ago

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

tyson andre

6 years ago

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

Nikita Popov

6 years ago

On Wed, Jul 1, 2020 at 8:55 PM tyson andre <tysonandre775@hotmail.com>
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

Nikita Popov

6 years ago

On Wed, Jul 1, 2020 at 9:05 PM Nikita Popov <nikita.ppv@gmail.com> wrote:

> On Wed, Jul 1, 2020 at 8:55 PM tyson andre <tysonandre775@hotmail.com>
> 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

tyson andre

6 years ago

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

Nikita Popov

6 years ago

On Wed, Jul 1, 2020 at 10:46 PM tyson andre <tysonandre775@hotmail.com>
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

tyson andre

6 years ago

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