PUT requests not handled correctly by the REST library #429

gotwarlost opened this Issue Aug 24, 2012 · 8 comments

4 participants

Yahoo Inc. member

from rest.common.js:

                if ('GET' === method) {
                    if (-1 === url.indexOf('?')) {
                        url += '?' + params;
                    } else {
                        url += '&' + params;
                } else if ('POST' === method) { /* WHAT ABOUT PUT? */
                    ioConfig.data = params;

This bug is not present in 0.3.26 and was introduced in 0.3.27. It is affecting our capability to move forward since we need PUT requests to a backend webservice to work.


Next Sprint candidate. Sprint's already pretty full though -- might get bumped. How critical is this? What's it blocking?


A proper REST implementation should support GET, POST, PUT, and DELETE. For performance reasons it's sometimes useful to also support HEAD requests. As part of that support HTTP status codes other than 200 can indicate success and should be properly handled. We may want to open a separate issue for full REST support so we can focus this bug on only what's being blocked at the moment tho.

Yahoo Inc. member

agree with @mojit0 - currently we are unable to move forward with our mojito dependency and have to pin ourselves to v0.3.26

This particular fix should be a one-line change (i.e. add a clause for PUT in addition to POST) - the effort is more on writing a unit test for this.


Next Sprint, S41.


Looks like this regression was added by #196 which was a fix for a YUI 3.5.1 regression. If we're upgrading to YUI 3.6 we can roll back #196 because it was fixed with IO.


Upgrade to YUI 3.6 is blocked by a regression, so we're waiting until 3.6.1 or 3.7.0 before we upgrade. Therefore, I am fixing this in place as it is now. PR incoming.

@mridgway mridgway pushed a commit to mridgway/mojito that referenced this issue Aug 29, 2012
Michael Ridgway Fixed Issue #429: Added params to body for PUT requests 5a2e0a8

Fixed in #450.

@mridgway mridgway closed this Aug 29, 2012
@mridgway mridgway was assigned Aug 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment