Protocol selection fails on POST when REST is enabled

Bug #1442710 reported by Stéphane Bisinger
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
WSME
Fix Released
Undecided
Unassigned

Bug Description

With the fix to bug #1419110 it is now impossible to select a protocol other than REST when you do a POST. This simple unit test demonstrates the issue (I put it in wsme/tests/test_root.py):

    def test_protocol_selection_post_method(self):
        import wsme.rest.protocol
        import wsme.protocol

        class P(wsme.protocol.Protocol):
            name = "test"

            def accept(self, r):
                return True

        root = WSRoot()
        root.addprotocol(wsme.rest.protocol.RestProtocol())
        root.addprotocol(P())
        from webob import Request
        req = Request.blank('/test?check=a&check=b&name=Bob')
        req.headers['Content-Type'] = 'test/fake'
        req.method = 'POST'
        p = root._select_protocol(req)
        assert p.name == "test"

Tags: rest soap
Revision history for this message
Chris Dent (cdent) wrote :

Hmm, that's unfortunate.

As a short term workaround the test can be made to work by adding the RestProtocol last.

Unfortunately the changes I made for #1419110 raise ClientSideErrors in accept that this appears to break the protcol handling. One option might be to move the media_type_accept call into iter_calls on the RestProtocol and change accept back to how it was before. I'll investigate this.

Is protocol stacking the common way of doing things with WSME?

(BTW none of this is Made Easy from my perspective...)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to wsme (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/172520

Revision history for this message
Chris Dent (cdent) wrote :

The review linked in the previous comment is a failing effort to try to get it working to satisfy both cases. We're going to need input from people wise about WSME.

Revision history for this message
Chris Dent (cdent) wrote :

I'm beginning the thing to do here is change addprotocol so that it raises a warning if addprotocol is called and the rest protocol is already in the stack.

Or even something more drastic like manage the list so that REST is always at the end. I can't figure out another way to preserve the accept handling changes which are _necessary_ in order to properly do HTTP.

Louis Taylor (kragniz)
Changed in wsme:
status: New → Confirmed
Revision history for this message
Stéphane Bisinger (kjir) wrote :

Here is my suggestion based on the code under review on Gerrit. This should work correctly for all cases, or at least all unit tests pass...

P.S.: I have no idea what the procedure is to contribute these suggestions on a WIP through Gerrit, I found no mention of it on the Developer's Guide... So I decided to attach the patch here!

Revision history for this message
Chris Dent (cdent) wrote :

Go ahead and send a new patchset up for review. After it is up, in the comments reference the one I made.

If yours works properly we'll just abandon the strawman I proposed.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to wsme (master)

Reviewed: https://review.openstack.org/172520
Committed: https://git.openstack.org/cgit/stackforge/wsme/commit/?id=e31045e57a102182d33252e3a6b07ddfa9488ebe
Submitter: Jenkins
Branch: master

commit e31045e57a102182d33252e3a6b07ddfa9488ebe
Author: Chris Dent <email address hidden>
Date: Fri Apr 10 18:41:32 2015 +0100

    Multiple protocol accept or content-type matching

    The changes in 8710dabb652dae775dee31789e91608f832e62e6 broke
    protocol failover when the REST protocol is listed before others
    (see bug referenced below).

    This patch tries to solve both issues by trying to match accept over all
    the protocols, only giving up a 406 or 415 if all protocols fail, using
    the last failure as the error message.

    Related-Bug: #1419110
    Closes-Bug: #1442710
    Change-Id: I328a392151013c46207519c245213d5dec48ecc9

Changed in wsme:
status: Confirmed → Fix Committed
Changed in wsme:
milestone: none → 0.8.0
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.