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

ScriptManager.Evaluate method does not cast variables #16856

Open
emrahtokalak opened this issue Oct 9, 2024 · 11 comments
Open

ScriptManager.Evaluate method does not cast variables #16856

emrahtokalak opened this issue Oct 9, 2024 · 11 comments
Labels
Milestone

Comments

@emrahtokalak
Copy link
Contributor

emrahtokalak commented Oct 9, 2024

Describe the bug

In versions 1.8.X, I was able to record content with my javascript code without any problems using the scriptManager.Evaluate method. In my tests on 2.0.2, I needed to cast the variables I read using the deserializeRequestData() method, it worked without the need for a previous cast operation. When I did not cast, I got a Json.Serialization error.

> at OrchardCore.Modules.InvokeExtensions.InvokeAsync[TEvents,T1](IEnumerable`1 events, Func`3 dispatch, T1 arg1, ILogger logger)
> 12:59:44|Default|0HN785HNJ9J4T:00000003|OrchardCore.ContentManagement.DefaultContentManager|ERR|IContentHandler thrown from OrchardCore.ContentManagement.Handlers.ContentPartHandlerCoordinator by JsonException
> System.Text.Json.JsonException: The JSON value could not be converted to System.String. Path: $.Text | LineNumber: 0 | BytePositionInLine: 9.
>  ---> System.InvalidOperationException: Cannot get the value of a token type 'StartArray' as a string.
>    at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_ExpectedString(JsonTokenType tokenType)
>    at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
>    at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
>    at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
>    at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
>    --- End of inner exception stack trace ---
>    at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
>    at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
>    at System.Text.Json.JsonSerializer.ReadFromNodeAsObject(JsonNode node, JsonTypeInfo jsonTypeInfo)
>    at OrchardCore.ContentManagement.Handlers.ContentPartHandlerCoordinator.InvokeHandlers[TContext,TFieldContext](TContext context, TFieldContext fieldContext, Func`4 partHandlerAction, Func`4 fieldHandlerAction, Boolean createPartIfNotExists, Boolean createFieldIfNotExists)

Orchard Core version
2.0.2

This used to work with 1.8.x, so it's a regression

To Reproduce

To simulate this error, I created a workflow and I am sending form-data to this workflow via postman, I am trying to read the values ​​with the deserializeRequestData method in the script task and create new content.

Workflow-Jint-Test.zip

`curl --location 'https://localhost:5001/workflows/Invoke?token=CfDJ8GpqN7JdvYREmCoyoRWr4NG4VFxf7K_ociCaKEGfCHAav8QbiCKSdMOTs93vPanX3uGA9jHCTSXb861mScAdsdk1LlhB7CQwtjzUMn1nCixxydStI_mAS89xaDWbfsJ_IX26FeLzfe9Pp83CzZiWmFzAxWZaFKHQ-v4Z15iA-dvAILSAtxOeLB7hBz4ZWxjxOtucrB7WgIcWzm2LugumzPv6kBYVV6TDkfhFp1I-n9RD' \
--header 'Content-Type: multipart/form-data' \
--header 'Authorization: Bearer eyJhbGciOiJSUzI1NiIsImtpZCI6IjNCMEJDREZFMzE5NEM1MzA2NURGMzBBRkI5OUUzQUIyRjM2QURBQzgiLCJ4NXQiOiJPd3ZOX2pHVXhUQmwzekN2dVo0NnN2TnEyc2ciLCJ0eXAiOiJhdCtqd3QifQ.eyJpc3MiOiJodHRwczovL2xvY2FsaG9zdDo1MDAxLyIsImV4cCI6MTcyODQ3MDkwOSwiaWF0IjoxNzI4NDY3MzA5LCJhdWQiOiJvY3Q6RGVmYXVsdCIsImp0aSI6ImZhODhhODI0LWJiNDAtNGZkMy1iZGMyLTFjZGIwZTY2ZjVjZiIsIm9jOmVudHlwIjoiYXBwbGljYXRpb24iLCJzdWIiOiIzZjk3MWY2ZGQxMGQ0ZWIzODFmM2Q3NTFhMTU0M2ZlMiIsIm5hbWUiOiJFbXJhaCBQb3N0bWFuIENsaWVudCIsInJvbGUiOiJSZXN0Q29udGVudEFwaVJvbGUiLCJQZXJtaXNzaW9uIjpbIkFjY2Vzc0NvbnRlbnRBcGkiLCJBcGlWaWV3Q29udGVudCIsIkNsb25lQ29udGVudCIsIkNsb25lT3duQ29udGVudCIsIkRlbGV0ZUNvbnRlbnQiLCJEZWxldGVPd25Db250ZW50IiwiRWRpdENvbnRlbnQiLCJFZGl0T3duQ29udGVudCIsIkVkaXRDb250ZW50T3duZXIiLCJMaXN0Q29udGVudCIsIlByZXZpZXdDb250ZW50IiwiUHJldmlld093bkNvbnRlbnQiLCJQdWJsaXNoQ29udGVudCIsIlB1Ymxpc2hPd25Db250ZW50IiwiVmlld0NvbnRlbnQiLCJWaWV3T3duQ29udGVudCIsIkV4ZWN1dGVHcmFwaFFMTXV0YXRpb25zIiwiRXhlY3V0ZUdyYXBoUUwiXSwib2lfcHJzdCI6IjNmOTcxZjZkZDEwZDRlYjM4MWYzZDc1MWExNTQzZmUyIiwiY2xpZW50X2lkIjoiM2Y5NzFmNmRkMTBkNGViMzgxZjNkNzUxYTE1NDNmZTIiLCJvaV90a25faWQiOiJlMzhlODg1MjMyZDg0MzQ2YTE3YmU2ZjhhNTliZDEwNyJ9.OvAxmJl47FoQMngoeehpmI-ezAcf1uWwPZDsfllbES-tpr2suwV_1mn5a_fPlt_kdLU8MJ_ka2dYgK0iSI_UzFgWFkyJu5Q9SBCB9K9nOwZsXxQ3-BIurlbOZRbK_c-rXWqSLnGR9qdzohBS-Rz-e83ZHTEPXXLyLtDzWhFi7pXZJM0bEolqRIgDhysPg-PsicahxcZoHG_bA-VEVfXZWUUF1PA9HLz3dqFgCgDk0pDcitg4EReIlRxY0g5dpAZe8bDMmUulaZq48JKMhPzi4j51a2pkniyhkf7GTTj3P-fTFPZP3N0tc9ahycStjOlhw31w-pjlsN-iVx9zeuSfCQ' \
--form 'Title="Jint Test Postman"' \
--form 'Name="Emrah"' \
--form 'Age="37"' \
--form 'CreateDate="2024-10-09T00:00:00Z"'`

Expected behavior

I expect it to work with this js;

var data = deserializeRequestData();

createContentItem("JintTestModel", true,
    {
        "JintTestModel": {
            "Name": {
                "Text": data.Name
            },
            "Age": {
                "Value": data.Age
            },
            "CreateDate": {
                "Value": data.CreateDate
            }
        },
        "TitlePart": {
            "Title": data.Title
        }
    });

After the 2.x upgrade, it only works properly as I cast it below.

var data = deserializeRequestData();

createContentItem("JintTestModel", true,
    {
        "JintTestModel": {
            "Name": {
                "Text": String(data.Name)
            },
            "Age": {
                "Value": Number(data.Age)
            },
            "CreateDate": {
                "Value": String(data.CreateDate)
            }
        },
        "TitlePart": {
            "Title": String(data.Title)
        }
    });

Is this an expected situation? Should I add cast functions to all my codes like this after the upgrade?

@sebastienros
Copy link
Member

I tried to blame @gvkries but he said it was because of @lahma

@lahma
Copy link
Contributor

lahma commented Oct 10, 2024

Sounds like yet another STJ interop problem, I guess behavior could have changed after switch from Newtonsoft. Jint could also have some regression if no test coverage exists for this kind of thing.

@gvkries
Copy link
Contributor

gvkries commented Oct 13, 2024

This could be due to the way deserializeRequestData handles the form data. I've tried to reproduce the error, but it works for me. Only difference, I used application/x-www-form-urlencoded instead of multipart data.

@emrahtokalak
Copy link
Contributor Author

This could be due to the way deserializeRequestData handles the form data. I've tried to reproduce the error, but it works for me. Only difference, I used application/x-www-form-urlencoded instead of multipart data.

Are you saying that in version 2.0.2, we can use the data returned from the deserializeRequestData method without any casting? I'm getting all kinds of errors in my tests. I think the problem is not with the deserializeRequestData method, but with the Json Serializer.

@gvkries
Copy link
Contributor

gvkries commented Oct 14, 2024

Not sure what has changed here in 2.0, but I think the issue is that deserializeRequestData returns a dictionary with StringValues in the values. That's why you get a System.InvalidOperationException: Cannot get the value of a token type 'StartArray' as a string error. So I think you must cast to the correct type or handle this in another way.

@emrahtokalak
Copy link
Contributor Author

Not sure what has changed here in 2.0, but I think the issue is that deserializeRequestData returns a dictionary with StringValues in the values. That's why you get a System.InvalidOperationException: Cannot get the value of a token type 'StartArray' as a string error. So I think you must cast to the correct type or handle this in another way.

Given that casting was not previously required in the script, I'm hesitant to state definitively that casting is now mandatory post-2.0. This could potentially result in a negative user experience.

@gvkries
Copy link
Contributor

gvkries commented Oct 14, 2024

I agree that this seems to be a regression.

@emrahtokalak
Copy link
Contributor Author

To maintain compatibility with my existing scripts and minimize the need for casting, I've developed a custom method. This method implicitly converts non-string data types to strings to prevent runtime errors. However, explicit casting of non-string data types remains essential for correct data handling.

 _formAsJsonObject = new GlobalMethod
            {
                Name = "requestFormAsJsonObject",
                Method = serviceProvider => (Func<Dictionary<string, object>>)(() =>
                {
                    var httpContextAccessor = serviceProvider.GetRequiredService<IHttpContextAccessor>();
                    var sanitizer = serviceProvider.GetRequiredService<IHtmlSanitizerService>();

                    var formData = httpContextAccessor.HttpContext.Request.Form;

                    var result = new Dictionary<string, object>();

                    foreach (var field in formData)
                    {
                        var sanitizedValues = field.Value.Select(value => sanitizer.Sanitize(value)).ToArray();

                        if (sanitizedValues.Length == 1)
                        {
                            result[field.Key] = sanitizedValues[0];
                        }
                        else
                        {
                            result[field.Key] = sanitizedValues;
                        }
                    }

                    return result;
                })
            };

The ideal usage should be as follows: I have come to the conclusion that we should cast it in every way.

var data = deserializeRequestData();

createContentItem("JintTestModel", true,
    {
        "JintTestModel": {
            "Name": {
                "Text": String(data.Name)
            },
            "Age": {
                "Value": Number(data.Age)
            },
            "CreateDate": {
                "Value": String(data.CreateDate)
            }
        },
        "TitlePart": {
            "Title": String(data.Title)
        }
    });

@gvkries
Copy link
Contributor

gvkries commented Oct 14, 2024

I think we should keep this opened.

@gvkries gvkries reopened this Oct 14, 2024
@sebastienros
Copy link
Member

I think we need a Jint type handler for StringValues which would return a string or a string[] based on the number of values.

@sebastienros sebastienros added this to the 2.x milestone Oct 17, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

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

No branches or pull requests

4 participants