suds-community / suds

Suds is a lightweight SOAP python client for consuming Web Services. A community fork of the jurko fork.
https://suds.readthedocs.io/
GNU Lesser General Public License v3.0
172 stars 54 forks source link

Fix to avoid the randomness of the namespacing #31

Open AvnerCohen opened 4 years ago

AvnerCohen commented 4 years ago

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 - https://github.com/cackharot/suds-py3/issues/41

phillbaker commented 4 years ago

Thanks! Would you be able to pull some of the examples from the issue/SO into a test for this?

On Mon, Jan 27, 2020 at 12:36 PM Avner Cohen notifications@github.com wrote:

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 https://github.com/cackharot/suds-py3/issues/41

You can view, comment on, or merge this pull request online at:

https://github.com/suds-community/suds/pull/31 Commit Summary

  • Fiz to avoid the randomness of the namespacing

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/suds-community/suds/pull/31?email_source=notifications&email_token=AAAXCKPQH2C6U6OWYZ74YH3Q74LR7A5CNFSM4KMFHW7KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4II7XB7A, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAXCKND3FS5Q4ZMAJAH4BLQ74LR7ANCNFSM4KMFHW7A .

AvnerCohen commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

AvnerCohen commented 4 years ago

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

phillbaker commented 4 years ago

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

https://github.com/suds-community/suds/blob/7deb66af98ec102c9e23a7fc11f0868de3237b63/tests/test_request_construction.py#L445-L446

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 commented 4 years ago

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 - https://github.com/suds-community/suds/blob/7deb66af98ec102c9e23a7fc11f0868de3237b63/tests/test_request_construction.py#L445-L446 ?

phillbaker commented 4 years ago

Is this exact scenario covered here

Right, that's why I'm confused!

AvnerCohen commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.