wsgi_application.py not handeling URL prefixing correctly

[George Marshall]

Please consult answer #7
https://answers.launchpad.net/ladon/+question/168082

[jsgaarde (jakob-simon-gaarde)]

Hi George.

I tried your patch, but it messes up urls when using Apache2 and mod_wsgi. This is probably the most important webserver to support. I will debug tomorrow to see if I can fix it. Does your setup use the LADON_ROOT_PATH_INFO, REQUEST_URI or PATH_INFO while probing the client path?

Best regards
Jakob Simon-Gaarde

[jsgaarde (jakob-simon-gaarde)]

Hi George.

I had to change the indentation one place in order to make your contribution work with Apache2. Otherwise unchanged. Can you check that it still works in your setup?

And thanks for spotting obsolete imports :-)

http://bazaar.launchpad.net/~ladon-dev-team/ladon/ladon/revision/42

Best Regards
Jakob Simon-Gaarde

[George Marshall (george.marshall)]

Hi Jakob,

My patch will look at the environ and will prefer REQUEST_URI over PATH_INFO. At the same time it uses urlparse and ParseResult to edit the URL. If LADON_ROOT_PATH_INFO is present then it will prefix the path of the URL received by REQUEST_URI/PATH_INFO.

My only testing so far has been running the app inside of Django and standalone using uWSGI which hasn't caused any problems. I only filed the bug because it contained the original patch you had given me and that was problematic. Try it out with my sample project, posted in the Q&A.

[George Marshall (george.marshall)]

Hi Jakob,

You last commit still creates the pathing issue I was having. I branched the trunk and modified the code and linked it to this ticket.

[jsgaarde (jakob-simon-gaarde)]

Hi George.
I was a little tired and too fast when I did my last commit, and I actually missed some serious issues.

I also have a few comments to your contribution that have to be addressed, sorry :-) :

1. LADON_ROOT_PATH_INFO is the user's possibility to force through a specific client-path when all other means of probing either fail or give the wrong result, so it _must_ take precedence over REQUEST_URI and PATH_INFO.

2. You have removed the sname parameter from probe_client_path(), that won't work when LADON_ROOT_PATH_INFO is set, cause when it is set the service must ignore all other probing possibilities and add the service name using the detected name from parse_environ()

3. About the REQUEST_URI (your branch still doesn't work with Apache2) - I think I can see what is going wrong. Some webservers define REQUEST_URI as absolute and others as relative:
http://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Examples_of_absolute_URIs . So you will have to examine REQUEST_URI if it is relative or absolute to find out if you need to "decorate" url_dict with schema, netloc and port info.

4. You have to do more port detection, so that if the variable that delivers information about the client_path is not absolute (and therefor doesn't have port information), the client will be thrown to port 80 and 443. You have to do like I did before cause not all webservers deliver port information with the HTTP_HOST variable.

I have made a version now, that handles most of these comments - I'd like you to review it and see if it works. I haven't commited it yet but just tested the probing function in an isolated module:

http://paste.ubuntu.com/673052/plain/

Best Regards
Jakob Simon-Gaarde

[George Marshall (george.marshall)]

Hi Jakob,

So after some more tinkering around it appears the original issue was directly related to Ladon's URL reconstruction. I used wsgiref.util.request_uri for URL reconstruction instead, which solves the original issue completely. As a result I've opted to remove setting the ladon_root_path all together as it is no longer needed.

http://www.python.org/dev/peps/pep-0333/#url-reconstruction

Some potential issues may exist with HTTP_LADON_PROXY_PATH, I've yet to setup an environment to test this. On a simular note because URL reconstruction is fixed any issues with mod_wsgi or other WSGI server should be non-existent.

Also wsgiref requires Python >= 2.5 so I wasn't sure if you where trying to maintain any 2.4 compatibility.

[jsgaarde (jakob-simon-gaarde)]

Hi George.

OK, I'm glad you got it working :-) Did you look at my proposal: http://paste.ubuntu.com/673052/plain/ that still takes offset in your suggested changes? It should actually work with your prior setup, and most other setups I can think of.

The problem as I see it is that there are a lot of webservers in the world and it is not 100% standardized how server variables are defined. Therefore it is up to Ladon to be tolerant and smart.

To your last question: Ladon is supported on Python 2.6 ->

Best Regards
Jakob Simon-Gaarde

[George Marshall (george.marshall)]

Hi Jakob,

I did take a look at your example, which is kind of what wsgiref.util.request_uri does. It's refined a lot more, making the above a case of reinventing the wheel.

[jsgaarde (jakob-simon-gaarde)]

Hi George.

But have you build it into Ladon, cause if it is a better solution than mine, I would prefer that instead.

Best regards
Jakob Simon-Gaarde

[George Marshall (george.marshall)]

Hi Jakob,

Yes, the changes have been integrated into ladon and tested. Look at my branch attached to this report.

[jsgaarde (jakob-simon-gaarde)]

Hi George.

I have tested your patch with both Python2 and Python3 and using the HTTP_LADON_PROXY_PATH variable. Everything seems to work nicely. The path prettyfying fix in generate_catalog_html and generate_service_html can however not be removed yet, it has to do with custom skins on the API Browser.

Everything else is applied and you are credited in the code. Thanks!

Best regards
Jakob Simon-Gaarde

[George Marshall (george.marshall)]

Hi Jakob,

How well tested is ladon? I keep finding bugs all over the place and digging through the unit test it looks like coverage is scarce.

I did find a missing variable definition in the wsgi_application.py and fixed that. I had also found another bug with the SOAP description changing my_var to my-var but, not converting back. This issue has been fixed and posted to my branch.

[jsgaarde (jakob-simon-gaarde)]

Hi George.

I do testing like 80% of the time while developing. Unfortunately I don't have time to update the automated testing framework all the time, cause setting up specific tests take time too, I do save these tests on my local system and hope to add them to the unit testing framework at some point. This is an open source project done in my free time, so some testing is up to the those users who choose to use it.

You are right about the SOAP request handler missing the '_' to '-' conversion - it was there before I re-wrote the whole request parser - thanks. I missed that test, besides that it has been pretty thouroghly tested:
* Unbound arguments of primitives
* Arguments of Complex Types
* Arguments with nested Complex Types
* Arguments with nested array based Complex Types
* Arguments with primitive arrays
* and more

The missing variable you found is actually not missing - something must have gone wrong when you merged. Take a look at the trunk repository and you will see that the fix_path = urlparse(client_path) (line) you found missing is actually in the trunk version at the time you merged:
http://bazaar.launchpad.net/~ladon-dev-team/ladon/ladon/view/head:/frameworks/python/src/ladon/server/wsgi_application.py#L190

Have you found more bugs or what do you mean by "all over the place"?

Best regards
Jakob Simon-Gaarde

[jsgaarde (jakob-simon-gaarde)]

IHi George.

I just tested your changes to the SOAP interface - they are looking good. I have merged them into the trunk.

The link from previous comment has changed because of the commit:
http://bazaar.launchpad.net/~ladon-dev-team/ladon/ladon/view/48/frameworks/python/src/ladon/server/wsgi_application.py#L190

/ Jakob

[George Marshall (george.marshall)]

Hi Jakob,

My apologies for the overstatement about "bugs all over the place." I was frustrated with the SOAP interface not working correctly because of the conversion issue. At the same time I was also trying to run through the ladon tests suite, which as explained doesn't include all your local tests as a sanity check.

[George Marshall (george.marshall)]

Hi Jakob,

I pushed some more changes to my branch. This time fixing the issue of param stacking when selecting a template skin.

[jsgaarde (jakob-simon-gaarde)]

Hi George.

Great job, I have tested with ladonctl and apache2, Python 2 and Python 3.
All changes have been merged into the main branch.

Best regards
Jakob Simon-Gaarde