WsdlToPhp / PackageGenerator

Generates a PHP SDK based on a WSDL, simple and powerful, WSDL to PHP
https://providr.io
MIT License
425 stars 73 forks source link

Improper validation of array #274

Closed awaters-pa closed 1 year ago

awaters-pa commented 2 years ago

Describe the bug I have generated a package from this WSDL: https://cs-training.test.seatgeekenterprise.com/weblink/19995/get.resource/wsdl/all

One of the data types generated is derived from AbstractStructArrayBase. It has a method that takes an array of strings as a parameter. Inside that method the parameter is being validated using preg_match. The issue is that the parameter is an array and it is being passed directly to preg_match. This is failing due to the parameter of preg_match being required to be a string. The method is called setGuid in the attached class.

ArrayOfguid.txt

mikaelcom commented 2 years ago

It indeed seems that the generation for the pattern is missing the fact that the value is an array. It'll be fixed as soon as possible

takuy commented 1 year ago

Running into this issue, any fix available?

mikaelcom commented 1 year ago

Hi, No fix available yet, sorry. I must take some time to fix this issue. If anyone is confident enough to update the required validation rules within the Validation folder, feel free to create a PR :smile:

takuy commented 1 year ago

Hi, No fix available yet, sorry. I must take some time to fix this issue. If anyone is confident enough to update the required validation rules within the Validation folder, feel free to create a PR 😄

I did some digging. I think it's just this line here....? The proper rules are already getting applied by the if-else chain before it. Is that supposed to be in the final elseif instead? Or in a final else? https://github.com/WsdlToPhp/PackageGenerator/blob/5b52d002a5b51adf475b0eec788032765a109f36/src/File/Validation/Rules.php#L48

mikaelcom commented 1 year ago

@takuy I think it's more an issue that should be handled at https://github.com/WsdlToPhp/PackageGenerator/blob/develop/src/File/Validation/AbstractSetOfValuesRule.php#L66-L70, so the pattern and other applicable validation rules should be applied within the foreach and not in the setter itself on the passed array. I'll try to fix it

mikaelcom commented 1 year ago

Hi @awaters-pa , I'm trying to improve the way the project is managed.

I created a team for reviewers "only" members so if you accept the invitation, you should be able to review and validate, if it's ok for you, the PR #279.

You can test the PR using one of these phars:

You let me know

awaters-pa commented 1 year ago

Hi There @mikaelcom

This is testing out real well. There is a change I'm curious about though. Not sure if this is related to this change or another so let me know if I need to ask it somewhere else.
In the generated class for AddPaymentToBasket_Request in the method setPaymentDescription there is a change that converts the passed in parameter to a SoapVar. The original code, just assigned the passed in parameter to the property. What is the purpose of this chage?

mikaelcom commented 1 year ago

In the generated class for AddPaymentToBasket_Request in the method setPaymentDescription there is a change that converts the passed in parameter to a SoapVar. The original code, just assigned the passed in parameter to the property. What is the purpose of this chage?

Nothing in the generator generates a SoapVar-typed variable. Can you paste the generated code?

awaters-pa commented 1 year ago

In the generated class for AddPaymentToBasket_Request in the method setPaymentDescription there is a change that converts the passed in parameter to a SoapVar. The original code, just assigned the passed in parameter to the property. What is the purpose of this chage?

Nothing in the generator generates a SoapVar-typed variable. Can you paste the generated code?

I might have had something messed up locally in the last run. I'm running it again and will let you know what I find.

awaters-pa commented 1 year ago

After looking over this it seems that our previous engineer had to change a few of the property assignments to use SoapVar instead of a straight assignment. I'll have to dig into why as they are no longer with us. For now though, it seems like the original issue reported is resolved. Thank you for your work on this!

mikaelcom commented 1 year ago

Thanks for your feedback!

mikaelcom commented 1 year ago

Are you able to approve the PR #279? I can validate it anyway, it's just for the process :smile:, thx!