Avoid the stack trace being part of the Error exeptions message

[johannes85]

Using the original cause of the TargetInvocationException as message for Error caused the stack trace to be leaked to the browser, even when in the prod environment, since setting the message executes toString on the original Exception.

This change uses only the message of the original exception to set the message of the Error object. The full exception is set as cause for displaying the stack trace later (in xp-forge/web).

Also see: https://github.com/xp-forge/web/pull/59

[thekid]

I'm not sure what you're trying to do here:

Demo application

<?php

use lang\IllegalAccessException;
use web\Application;

class Raise extends Application {

  public function routes() {
    return function($req, $res) {
      throw new IllegalAccessException('Go away:)');
    };
  }
}

xp -supervise web Raise

xp -supervise web -p prod Raise

---

This looks exactly as expected to me.

[johannes85]

I don’t know why you can’t reproduce the issue but this lead to a security issue in production. The message contained the stack trace because it was set to the exception object itself not only the message. The regression test „error_exclude_stacktrace_in_messag“ can be used to reproduce it.

[johannes85]

@thekid Ok, now I know why you couldn't reproduce it:

You didn't tested the problem with a frontend handler. The problem is in frontend not in the web library. The change in the web library is to display a nicer stacktrace but the actual problem is here in this lib.

This code leads to the exception in $e->getCause() to be converted to a string which will also contain the stacktrace:

throw new Error(500, $e->getCause());

To fix it, only the cause message has to be used as message for the Error and also the cause has to be set to $e->getCause() so the stacktrace can be displayed in the error outputted by the web lib:

$cause= $e->getCause();

...

throw new Error(500, $cause->getMessage(), $cause);

The change in the web lib only displays the stack trace of the cause not of the actual Error exception since this isn't what the developer need to debug its code. Also the cause stacktrace gets cutted way to early since it is very long with the stacktrace of the Error exception prepended (see last screenshot).

Proof

App.class.php

<?php

use web\Application;
use web\frontend\{Frontend, Templates};
use web\handler\FilesFrom;
use io\Path;

class App extends Application {

  /** @return [:var] */
  protected function routes() {
    return [
      '/' => new Frontend(
        new TestHandler(),
        new class() implements Templates {
          public function write($name, $context, $out) {}
        }
      )
    ];
  }
}

TestHandler.class.php

<?php

use lang\XPException;

class TestHandler {

  #[@get('/')]
  public function test() {
    throw new XPException('Insert  Stacktrace here => ');
  }
}

Result

After applying this fix

[johannes85]

@OlafSeng FYI

[thekid]

To fix it, only the cause message has to be used as message for the Error and also the cause has to be set to $e->getCause() so the stacktrace can be displayed in the error outputted by the web lib:

Thanks, applied this in xp-forge/frontend@740f2c0

[thekid]

:shipit: Released in https://github.com/xp-forge/frontend/releases/tag/v0.7.1