rfc:throwable_string_param_max_len

This is an old revision of the document!


PHP RFC: throwable_string_param_max_len: Configurable string length in getTraceAsString()

Introduction

Since 2003, Throwable->getTraceAsString() and Throwable->__toString() have limited the length of string function arguments in stringified stack traces to 15 bytes (including lines such as #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 it when debugging an issue or during local development.

While 15 bytes may be a reasonable default for many use cases (e.g. allowing packing more stack frames on a screen or within a byte limit), it would be useful to be able to raise that default.

This hardcoded limit affects various places where exceptions and errors are converted to strings, such as:

  1. echo $throwable;
  2. log('something', $throwable->getTraceAsString());
  3. Uncaught Throwables that crashed an application.

Proposal

Add 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. (Changeable by PHP_INI_ALL)

A maximum value is enforced to make it harder to accidentally run out of memory or disk space (e.g. if long strings occur multiple times in a stack trace). Throwable->getTrace() can be used if the full argument values are needed.

Backward Incompatible Changes

None

Proposed PHP Version(s)

PHP 8.0

RFC Impact

To SAPIs

If the ini setting is not changed, there will be no impact.

If the user decides to raise the string length limit, then stack traces will contain longer representations of string params. This may result in more data being logged when Throwable->__toString() or Throwable->getTraceAsString() are used (e.g. full urls, full file paths, full file contents, etc). Stringified stack traces may also exceed what applications assumed the typical length would be (e.g. udp packet sizes when syslogging).

php.ini Defaults

To keep backwards compatibility for reasons such as Future scope: Raise the default value, and to ensure stack traces remain similar in development/production, all proposed values are 15.

  • hardcoded default value: 15
  • php.ini-development value: 15
  • php.ini-production value: 15

Open Issues

Make sure there are no open issues when the vote starts!

Unaffected PHP Functionality

Other ways to inspect stack traces such as debug_print_backtrace() and Throwable->getTrace() are not affected. They do not have string length limits.

Future Scope

Decrease the ini setting's minimum

Future RFCs may suggest allowing 0 in throwable_string_param_max_len. This RFC keeps a minimum of 15 bytes because application developers/users may prefer to have the same level of detail in bug reports if stringified exceptions are included in bug reports.

Being able to set the minimum value to 0 may have the benefit of avoiding accidentally exposing sensitive information in external dependencies or legacy applications. See Impact of raising string param length limit for examples of those issues.

Raise the default value

Since 2003, disk space, screen sizes, etc. have increased significantly. However, stack traces have probably also gotten longer in some frameworks, and the maximum syslog length may be limited to only a few thousand bytes on some platforms.

Application may be unexpectedly relying on the hardcoded limit of 15 to avoid logging sensitive information such as full urls, full paths, or full file contents.

Proposed Voting Choices

Add a new ini setting throwable_string_param_max_len as described in the RFC. (Yes/No, requiring 2/3 majority)

Poll

  • Informal poll: Interest in allowing it to be set to 0
  • Informal poll: Interest in raising the default setting value

Changelog

0.2: Add “Impact of raising string param length limit” section.

References

https://externals.io/message/110717 “Making the hardcoded string length limit of Throwable->getTraceAsString() configurable”

Appendix

Impact of raising string param length limit

For example, code such as the following already had multiple issues such as exposing $appSecret and the potential for XSS from echoing $rawUserInput without html escaping (e.g. <script>...</script>), and should be rewritten to stop doing that. If more than 15 bytes are output, the severity of that bug may be much higher (e.g. if $appSecret was -----BEGIN RSA PRIVATE KEY-----..., which was previously truncated to 15 bytes and would omit the private key itself).

function unsafeHTMLRenderingExample(string $rawUserInput, string $appSecret) {
   echo "<h1>Heading</h1>\n";
   try {
       process($rawUserInput);
   } catch (Exception $e) {
       // The output will include both $rawUserInput and $appSecret.
       // Previously, only 15 bytes would be displayed.
       echo "This should not happen: $e\n";  
   }
}

Static analyzers may be able to detect potentially unsafe uses of getTraceAsString() and __toString() in the future. Currently, though, I don't believe they check for this specific issue. https://psalm.dev/docs/security_analysis/ and https://gerrit.wikimedia.org/g/mediawiki/tools/phan/SecurityCheckPlugin/#mediawiki-security-check-plugin are projects in that area.

Because the default remains at 15 bytes, this RFC should not make unsafe code like this worse unless the ini setting is changed deliberately.

rfc/throwable_string_param_max_len.1593629229.txt.gz · Last modified: 2020/07/01 18:47 by tandre