CSRF Token value is empty on form failure

[drn5910]

When the middleware ConvertEmptyStringsToNull is enabled and you submit a form, redirect back to the form with errors (without input), the _token field no longer has a value. I have not tested redirecting back with input.

This has caused a pain in the backside as essentially if they fail the the form and there's no "old" value for the _token field, it fills it with an empty value. This then causes any further attempts to fail due to a TokenMismatchException.

I thought at first my middleware wasn't working but can confirm this issue simply by checking the value that's passed from the token method to the hidden method and the one that gets returned from the getValueAttribute for the _token field. The value is correct before but it's null afterwards.

A quick solution is to simply check at the top of getValueAttribute if the name is equal to the specified token field (e.g. _token).

Thank you.

[mlantz]

Thanks for catching this @david-ridgeonnet I'll put a fix in asap

[mlantz]

Based on what we're saying looks to me like the simplest would be to add to the getValueAttribute as you mentioned:

if ($name == '_token') {
     return $this->csrfToken;
}

[drn5910]

Indeed, or you could amend this:

public function getValueAttribute($name, $value = null)
    {
        if (is_null($name)) {
            return $value;
        }

to this:

public function getValueAttribute($name, $value = null)
    {
        if (is_null($name) || $name === '_token') {
            return $value;
        }

That way it returns the original value that the _token field was going to use.

However while writing this I had a thought, what if it isn't just the _token field that is affected?

If I have a form with a pre-filled value (e.g. just a standard text input) and submit the form. On the server, if I do extra validation and redirect back without specifying withInput() then that pre-filled value is now blank... I'm not sure this is the intended behaviour?

[mlantz]

Since the issue appears to be directly involved with _token I kinda think its best to just change that. Using this approach the token should always work since the return will have the app's current value of the token.

[drn5910]

Yes but I'm also saying I think there is a further issue. If I have a form input like so:

Form::text('test', 'My Default Value')

Submit the page, and then in the function handling the request, simply do:

return redirect()->back()->withErrors('Nope, not going to happen');

The current result means the form input will be rendered empty, without the My Default Value.
Prior to this 28ba758 it would have filled that value with the default.

This change has broken more than the _token, it's broken every pre-defined value and changed it to null when the request is returned with errors (but no old input).

[mlantz]

I found the core issue:

                && is_null($old)
                && !is_null($this->view->shared('errors'))
                && count($this->view->shared('errors')) > 0

needs to be

                && is_null($old)
                && is_null($value)
                && !is_null($this->view->shared('errors'))
                && count($this->view->shared('errors')) > 0

I have a test working replicating what youre saying above

[drn5910]

Sounds good to me � thanks for your time on this

[mlantz]

No problem - it actually all makes sense now - release coming in a few min

[drn5910]

Thanks - this has fixed.

However the change in c0dc174 where you added the:

        if ($name == '_token') {
            return $this->csrfToken;
        }

Will break when the csrfToken is not set (if you see the token() method, it gets the value correct. People have posted issues for this already (#339). Simple removing this change will fix it, as the other change you did in 4aae8fb solves the issue anyway.