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

Circular element references inside wsdl - assistance needed #1142

Merged
merged 2 commits into from May 14, 2021
Merged

Circular element references inside wsdl - assistance needed #1142

merged 2 commits into from May 14, 2021

Conversation

doubleaxe
Copy link
Contributor

Hello.

This fix #1101 fixed one bug and introduced another. Now parsing wsdl with recursive ref elements will cause stack overflow. I created 2 test cases which demonstrate this bug, but I need advice of someone who knows this project better than me.

I can quick fix stack overflow with one kludge - just like #1101, something like this, or maybe a little more complex - because second test will fail with this:

          if (!this.$ref) {
            definitions.descriptions.types[typeName] = elem;
          } else {
            const refType = splitQName((typeElement as ElementElement).$type);
            if(refType.name != typeName)
              definitions.descriptions.types[typeName] = elem;
          }

As pedantic person, I think kludge is not the way to do. In my opinion main problem is more deep in internals: both element names (with references) and type names go to the same object definitions.descriptions.types, they are mixed, and this introduces all these bugs with elements and types, but I'm not sure in this, because I'm new to node-soap internals, I just use it as a regular user.

So I need advice from someone, I can try to fix this by splitting definitions.descriptions.types for elements and for types, but it seems this is core internal feature, I'm afraid to break something and waste my time making PR which will not be accepted.

@coveralls
Copy link

coveralls commented May 3, 2021

Coverage Status

Coverage remained the same at 95.019% when pulling e588d8c on doubleaxe:recursive_wsdl_ref_bug into bf3b07d on vpulim:master.

@doubleaxe
Copy link
Contributor Author

I updated PR, this is my vision on how this bug should be fixed. All tests seems passing, I only doubt in backward compatibility.

@jsdevel jsdevel merged commit d684525 into vpulim:master May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants