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

Layers - Invalid javascript condition #12901

Closed
sobotama opened this issue Nov 29, 2022 · 3 comments · Fixed by #12968
Closed

Layers - Invalid javascript condition #12901

sobotama opened this issue Nov 29, 2022 · 3 comments · Fixed by #12968
Labels
Milestone

Comments

@sobotama
Copy link
Contributor

sobotama commented Nov 29, 2022

Discussed in #12894

Originally posted by sobotama November 28, 2022

Describe the bug

One of our users writes javascript conditions in layer rules for resolving URLs and once they mistyped and wrote the condition like this:

("test")

instead of this:

url("test")

After adding any widget to this faulty layer the whole frontend fails with code 500:

System.FormatException: String 'test' was not recognized as a valid Boolean.
   at System.Boolean.Parse(ReadOnlySpan1 value)
   at System.Boolean.Parse(String value)
   at System.String.System.IConvertible.ToBoolean(IFormatProvider provider)
   at System.Convert.ToBoolean(Object value)
   at OrchardCore.Rules.Services.JavascriptConditionEvaluator.EvaluateAsync(JavascriptCondition condition) in src\OrchardCore.Modules\OrchardCore.Rules\Services\JavascriptConditionEvaluator.cs:line 30
   at OrchardCore.Rules.ConditionEvaluator1.OrchardCore.Rules.IConditionEvaluator.EvaluateAsync(Condition condition) in src\OrchardCore\OrchardCore.Rules.Abstractions\ConditionEvaluator.cs:line 16
   at OrchardCore.Rules.Services.RuleService.EvaluateAsync(Rule rule) in src\OrchardCore.Modules\OrchardCore.Rules\Services\RuleService.cs:line 20
   at OrchardCore.Layers.Services.LayerFilter.OnResultExecutionAsync(ResultExecutingContext context, ResultExecutionDelegate next) in src\OrchardCore.Modules\OrchardCore.Layers\Services\LayerFilter.cs:line 113
the rest of stack is ommited.

Backend is still accesible and it can be repaired, however the client is unhappy, that the whole site can be broken this easily, even though they now use the proper URL condition.

To Reproduce

  1. Create a layer
  2. Add rule - Javascript condition with this script: ("test") - or any other mistake - url("test), ul("test")
  3. Assign any widget to the layer
  4. Go to anywhere in the frontend

Expected behavior

Maybe just not showing the affected layer and logging the error, while the rest of the site still functions normally

I tried it on OrchardCore versions 1.1, 1.4 and 1.5 with all of them behaving the same.

@sebastienros sebastienros added this to the 1.x milestone Dec 8, 2022
@sebastienros
Copy link
Member

  • validate the input by parsing it and displaying the syntax error
  • ensure that evaluating the script is not breaking the full page if this is syntactically correct but it fails at runtime (try with a javascript exception).

@sebastienros
Copy link
Member

@sobotama with my recommendation, could you try to provide a PR that fixes the issue?

@sobotama
Copy link
Contributor Author

@sebastienros sure, I'll do it

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

Successfully merging a pull request may close this issue.

3 participants