Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Errors" in templates leak server information even when !YII_DEBUG when requesting json #4468

Closed
Zae opened this issue Jun 29, 2019 · 5 comments

Comments

@Zae
Copy link

Zae commented Jun 29, 2019

Description

When an error occurs in a twig template (in my current case a "TypeError"), this is not caught by \craft\web\view@renderTemplate because this only catches RuntimeExceptions and not Errors.

This causes the Error to bubble up and rendered with full file path information by \craft\web\controller@asErrorJson

This is obviously not what I want to happen in production mode, I would expect the error to be hidden like the RuntimeExceptions are.

So either the error should be captured correctly by renderTemplate or the runAction method should not give the full error message to asErrorJson method and respect the YII_DEBUG flag and not report the full path unless it is set. Preferrably both :)

{
    "error": "Argument 2 passed to Twig\\TemplateWrapper::__construct() must be an instance of Twig\\Template, string given, called in /var/www/html/craft/vendor/twig/twig/src/Environment.php on line 359"
}

Steps to reproduce

  1. Run Craft with devmode = false and suppressTemplateErrors' => true
  2. Somehow trigger a Error not a RuntimeException in a view with the Accept: application/json header set in the request.
  3. See the full path of the file that the error occurred in.

Additional info

  • Craft version: 3.1.32.1
  • PHP version: 7.2.18
@brandonkelly
Copy link
Member

What exactly were you doing to trigger that error?

@Zae
Copy link
Author

Zae commented Jul 1, 2019

I'm not sure how I triggered this specific twig error, I seem to be having some problems with twig in this project. The original issue I was having was that there are two functions added to twig, one that creates a form and one that accepts that form to return a honeypot code (freeform)

However if the form does not exist, we still call the honeypot code with a null, and php does not accept this because it expects a Form instance not a null, however somehow I botched my twig locally to get this error.

But the actual error does not really matter. As long as there is an error that is not a RuntimeException craft will show the full path of the error in the json response, even when devmode is off.

The actual error I was getting that started this is:

{"error":"Argument 1 passed to Solspace\\Freeform\\Variables\\FreeformVariable::getHoneypot() must be an instance of Solspace\\Freeform\\Library\\Composer\\Components\\Form, null given, called in  /var/www/html/craft/vendor/twig/twig/src/Extension/CoreExtension.php on line 1687"}

@Zae
Copy link
Author

Zae commented Jul 1, 2019

If you want to test this out, install freeform.

Create a template called freeform.json and call it using something like postman with Accept: application\json

Give it these contents:

{% set name = craft.request.getParam('handle') %}
{% set form = craft.freeform.form(name) %}
{% set honeypot = craft.freeform.honeypot(form) %}

And give the ?handle param some form that does not exist, like xxx, this will cause a TypeError in the getHoneypot code and craft will show you in the json that is returned.

But anything that throws an error like that will do :)

@brandonkelly
Copy link
Member

Thanks! Fixed this for the next release.

To get the fix early, change your craftcms/cms requirement in composer.json to:

"require": {
  "craftcms/cms": "dev-develop#e31dd7c189ea16ceabeea7d4fc6813132633af1c as 3.1.32.1",
  "...": "..."
}

Then run composer update.

@Zae
Copy link
Author

Zae commented Jul 1, 2019

Tested it and it works, fix confirmed :D

Thanks for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants