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

Fix to avoid the randomness of the namespacing #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix to avoid the randomness of the namespacing #31

wants to merge 1 commit into from

Conversation

AvnerCohen
Copy link

This is taken from the fix from Alex Miro in this stackoverflow question:

https://stackoverflow.com/questions/10207167/python-suds-wrong-namespace-prefix-in-soap-request

Without this, when there is ComplexType in the xsd, the namespace creation is faulty and random.

The same issue is also repeated here - cackharot/suds-py3#41

@phillbaker
Copy link
Member

phillbaker commented Jan 27, 2020 via email

@AvnerCohen
Copy link
Author

@phillbaker I afraid I do not have enough understanding of the issue or suds to be able to cleanly recreate a repro and test.

What I can only share is that this is a result of an issue sending a soap request to this public wsdl:
https://community.workday.com/sites/default/files/file-hosting/productionapi/Recruiting/v33.2/Recruiting.wsdl
Linked from https://community.workday.com/sites/default/files/file-hosting/productionapi/Recruiting/v33.2/Recruiting.html

We could see that on some payloads, specifically on python3.7.4, the namespacing of the fields was created in a wrong way to generate an invalid XML.

I guess there are three options moving forward:

  1. Keep this PR open for someone who has enough understanding of the issue to create a clean repro.
  2. Close the PR.
  3. Merge it as it seems to have fixed the problem for others and me, with no impact on the existing test suite.

Would love to see this merged or picked up by someone who understands better what may have happened.

@phillbaker
Copy link
Member

@AvnerCohen sorry for the slow response here. In order for folks to take a further look can you share a simple code snippet that reproduces this problem? Especially, what service method in the linked wsdl is being called that generates this issue?

@phillbaker
Copy link
Member

To clarify, I think this is actually adding functionality to support complex types defined by the ref= attribute instead of the type= attribute (the latter is currently supported).

@AvnerCohen
Copy link
Author

@phillbaker I was hoping the provided wsdl could help with that, but I couldn't really create a lean and specific reproduction steps.
We have seen this in production but the complexity there makes it very hard to actually strip down a repro.

I also suspect that there is some complexity here around python version (py2 is had random dict order while py3 is more consistent). So it's a tricky one indeed.

@phillbaker
Copy link
Member

@AvnerCohen can you confirm the version and/or fork of suds you're using?

@AvnerCohen
Copy link
Author

This was tested with suds-community 0.8.4 - https://pypi.org/project/suds-community/0.8.4/

@phillbaker
Copy link
Member

Very odd - it looks like this test should be capturing this behavior:

<xsd:element ref="tns:local_referenced"/>
<xsd:element ref="second:external"/>

However, putting a pdb.set_trace() in the same spot as the suggested code, I don't see the debugger hit during the test.

Looking at the request output, I actually see that there is no namespace prefix.

@AvnerCohen I've looked at the wsdl you provided, and the only ref= usages are for version elements. Can you confirm the client service requests (ie the python calls) that were not working?

@AvnerCohen
Copy link
Author

I've noticed this in the notes of the other people facing this issue:
"""
From what I can tell from the WSDL it occurs when there is a "complexType" that is not part of the "targetNamespace" (it has it's own), which has a child that is also a complexType, but with no namespace set.
"""

Is this exact scenario covered here -

<xsd:element ref="tns:local_referenced"/>
<xsd:element ref="second:external"/>
?

@phillbaker
Copy link
Member

Is this exact scenario covered here

Right, that's why I'm confused!

@AvnerCohen
Copy link
Author

Maybe I am missing something, but I see only one xsd:complexType that is name="fRequest" , I'd expect name="fRequest" do have a child complexType that is complex.
What I see is that it has:
local - Which is a string.
tns:local_referenced - Which is a string
second:external - which is a string (external one, but still a string).

Or am I missing something?

@phillbaker
Copy link
Member

I added an additional case for the complex external on a branch in 251acb8.

You can pull it down and run just the test via: python -Wd setup.py test -a "tests/test_request_construction.py::test_element_references_to_different_namespaces" The debugger is still not getting hit.

@AvnerCohen
Copy link
Author

Thanks for the effort @phillbaker .
I downloaded this branch and tested both on production env and local, at this point, I could get this to reproduce.
I am not in a position right now to be able to test this on actual production traffic.
Suggest to close this or keep it for someone who can provide more info.

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

Successfully merging this pull request may close these issues.

2 participants