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

If there is a CR (LF) after a manual mode formula in a field, the PHP parser breaks. #3270

Closed
sburlot opened this issue Jun 21, 2019 · 7 comments
Closed
Assignees
Labels

Comments

@sburlot
Copy link

@sburlot sburlot commented Jun 21, 2019

BUG

If there is a CR (LF) after a manual mode formula in a field, the PHP parser breaks:
ie "%total% + %total2%\n" instead of "%total% + %total2%"
The MathParser\StdMathParser->parse('(90) + (0)\n') fails (see PHP error log)

The Javascript calculation works fine: no error.

trimming the formula in PHP should prevent this error

What Version Of Caldera Forms, WordPress and PHP Are You Using?

WordPress Version: 5.2.2, PHP Version: 7.2.19-0ubuntu0.18.04.1, MySQL Version: 5.7.26, Caldera Forms Version: 1.8.5, WP_DEBUG:

Does Your Issue Persist When You Disable All Other Plugins and Switch To The Default Theme?

Yes

What Is The Unexpected Behaviour?

Submitting the form does nothing (spinner in the middle of the screen)

What PHP Errors Have You Logged While Reproducing This Bug?

[21-Jun-2019 21:50:58 UTC] PHP Fatal error:  Uncaught Error: Call to a member function isTerminal() on null in [...]/wp-content/plugins/caldera-forms/vendor/mossadal/math-parser/src/MathParser/Parsing/Parser.php:166
Stack trace:
#0 [...]/wp-content/plugins/caldera-forms/vendor/mossadal/math-parser/src/MathParser/Parsing/Parser.php(124): MathParser\Parsing\Parser->shuntingYard(Array)
#1 [...]/wp-content/plugins/caldera-forms/vendor/mossadal/math-parser/src/MathParser/StdMathParser.php(55): MathParser\Parsing\Parser->parse(Array)
#2 [...]/wp-content/plugins/caldera-forms/classes/core.php(1560): MathParser\StdMathParser->parse('(90) + (0)\n')
#3 [...]/wp-includes/class-wp-hook.php(286): Caldera_Forms->run_calculation('90', Array, Array)
#4 [...]/wp-includes/plugin.php(208): WP_Hook->apply_filters('90', Array)
#5 [...]/wp-content/plugins/caldera-forms/classes/core.php(249 in [...]/wp-content/plugins/caldera-forms/vendor/mossadal/math-parser/src/MathParser/Parsing/Parser.php on line 166

What JavaScript Errors Have You Seen While Reproducing This Bug?

jquery-migrate.min.js?ver=1.4.1:2 JQMIGRATE: Migrate is installed, version 1.4.1
jquery.js?ver=1.12.4-wp:4 POST https://XXXX/cf-api/CF5ab4baa3c6576 500
send @ jquery.js?ver=1.12.4-wp:4
ajax @ jquery.js?ver=1.12.4-wp:4
request @ jquery-baldrick.min.js?ver=1.8.5:1
c @ jquery-baldrick.min.js?ver=1.8.5:1
(anonymous) @ jquery-baldrick.min.js?ver=1.8.5:1
dispatch @ jquery.js?ver=1.12.4-wp:3
r.handle @ jquery.js?ver=1.12.4-wp:3
caldera-forms-front.min.js?ver=1.8.5:1 Uncaught TypeError: Cannot read property 'data' of undefined
    at Object.error (caldera-forms-front.min.js?ver=1.8.5:1)
    at Object.request_error (jquery-baldrick.min.js?ver=1.8.5:1)
    at c (jquery-baldrick.min.js?ver=1.8.5:1)
    at Object.error (jquery-baldrick.min.js?ver=1.8.5:1)
    at i (jquery.js?ver=1.12.4-wp:2)
    at Object.fireWith [as rejectWith] (jquery.js?ver=1.12.4-wp:2)
    at x (jquery.js?ver=1.12.4-wp:4)
    at XMLHttpRequest.c (jquery.js?ver=1.12.4-wp:4)
error @ caldera-forms-front.min.js?ver=1.8.5:1
request_error @ jquery-baldrick.min.js?ver=1.8.5:1
c @ jquery-baldrick.min.js?ver=1.8.5:1
error @ jquery-baldrick.min.js?ver=1.8.5:1
i @ jquery.js?ver=1.12.4-wp:2
fireWith @ jquery.js?ver=1.12.4-wp:2
x @ jquery.js?ver=1.12.4-wp:4
c @ jquery.js?ver=1.12.4-wp:4
XMLHttpRequest.send (async)
send @ jquery.js?ver=1.12.4-wp:4
ajax @ jquery.js?ver=1.12.4-wp:4
request @ jquery-baldrick.min.js?ver=1.8.5:1
c @ jquery-baldrick.min.js?ver=1.8.5:1
(anonymous) @ jquery-baldrick.min.js?ver=1.8.5:1
dispatch @ jquery.js?ver=1.12.4-wp:3
r.handle @ jquery.js?ver=1.12.4-wp:3
jquery.js?ver=1.12.4-wp:4 XHR failed loading: POST "https://avsm.ch/cf-api/CF5ab4baa3c6576".
send @ jquery.js?ver=1.12.4-wp:4
ajax @ jquery.js?ver=1.12.4-wp:4
request @ jquery-baldrick.min.js?ver=1.8.5:1
c @ jquery-baldrick.min.js?ver=1.8.5:1
(anonymous) @ jquery-baldrick.min.js?ver=1.8.5:1
dispatch @ jquery.js?ver=1.12.4-wp:3
r.handle @ jquery.js?ver=1.12.4-wp:3

NGINX Error

89.217.147.58 - - [21/Jun/2019:23:50:58 +0200] "POST /cf-api/CF5ab4baa3c6576 HTTP/2.0" 500 2749 "https://XXXXX/inscription-a-la-journee-de-formation/" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_5) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.1 Safari/605.1.15"

##################

@Shelob9

This comment has been minimized.

Copy link
Collaborator

@Shelob9 Shelob9 commented Jul 9, 2019

trimming the formula in PHP should prevent this error
Yes. Also, we might pre-process to change line break encoding.

@Shelob9 Shelob9 added this to the 1.8.6 milestone Jul 9, 2019
@Shelob9 Shelob9 added the Bug label Jul 9, 2019
@New0

This comment has been minimized.

Copy link
Collaborator

@New0 New0 commented Jul 10, 2019

Hello @sburlot , thank you for sharing this issue. Could you give me a context where you are using the line break in a manual calculation ?

@sburlot

This comment has been minimized.

Copy link
Author

@sburlot sburlot commented Jul 10, 2019

@New0 I don't use a line break, but the person who created the manual calculation did add a line break after the formula, so it wasn't visible. And I suppose it didn't matter with a previous version of the plugin.
In short, I don't need to add a line break, but if there's one, it shouldn't break the parser.

@Shelob9

This comment has been minimized.

Copy link
Collaborator

@Shelob9 Shelob9 commented Jul 10, 2019

I don't think there is any case where it should be used. Calling trim before parsing would prevent this sort of accidental line break from causing issues.

I think this is a regression worth fixing.

@New0

This comment has been minimized.

Copy link
Collaborator

@New0 New0 commented Jul 11, 2019

Thank you @sburlot , @Shelob9

My first tests using %total% + %total2%\n showed that the calculation is not working on the form either ( rolling back to previous versions gave same result ).
In my case, the formula is saved to the database as %total% + %total2% n , breaking the line and keeping n.
Then the trimming that is performed for frontend at l215 classes/sync/calc.php doesn't work as there is an "n" alone left at the end of the string that breaks calculation.

That makes me think we should trim the formula before saving it to the database, that would prevent my error and the case where it is saved with the backslash, which makes the calculation work on frontend but fails with new parser. ( we can also trim the formula before sending it to the new parser ).

@sburlot

This comment has been minimized.

Copy link
Author

@sburlot sburlot commented Jul 11, 2019

@New0 I don't know when the line break started to break the parser, I just assumed it was working with the previous release.
I second your suggestion but I would do both, i.e. trim before saving and trim before sending to the parser, in case old formulas were saved with a line break.
If you have a way to trim all stored formulas when an update of the plugin is made, then not trimming before sending to the parser could save a few ms.
These are just ideas, you surely know better than me.

@New0

This comment has been minimized.

Copy link
Collaborator

@New0 New0 commented Jul 23, 2019

Closed via 5728028

@New0 New0 closed this Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.