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

HTTP PATCH support for Wt 3.7 #104

Closed
wants to merge 3 commits into from
Closed

Conversation

jamal-fuma
Copy link

This patchset adds support for making Http Patch requests using Wt::Http::Client.
This patchset allows WResource to respond to Http Patch requests.
This patchset expands Wt::Http::Redirection support to allow for Method preserving redirection - e.g. Http 307/308 with a POST results in a Post.
This patchset expands Wt::Http::Redirection support to allow for Method translating redirection - e.g. Http 301/302 with a POST results in a Get.

@emweb
Copy link
Collaborator

emweb commented Feb 14, 2017

When automatic redirection was implemented, a conscious decision was made only to automatically redirect GET requests, and leave the decision to follow redirects on POST, PUT and DELETE to the user of the API. I'm not sure if we should do this automatically for POST, PUT and DELETE requests.

See this section in the RFC: https://tools.ietf.org/html/rfc2616#section-10.3.8
"If the 307 status code is received in response to a request other than GET or HEAD, the user agent MUST NOT automatically redirect the request unless it can be confirmed by the user, since this might change the conditions under which the request was issued."

Regards,
Roel

@jamal-fuma
Copy link
Author

The redirect for non GET requests should really have been in a separate pull request, I wanted this one to be PATCH support.

Nevertheless the RFC does indeed say a user agent MUST NOT redirect unless confirmed. However the current client doesn't warn on 307/308 redirects, and doesn't follow the redirect, which breaks some of my uses of Wt::Http::Client.

Perhaps an additional method - followRedirectsWithoutConfimation(bool) would be sufficient

@jamal-fuma
Copy link
Author

The original pull request without redirection support is #88

@emweb
Copy link
Collaborator

emweb commented Apr 13, 2017

PR #88 wouldn't properly merge. I've done it manually instead and will push the changes soon.

@emweb emweb closed this Apr 13, 2017
@jamal-fuma
Copy link
Author

Brilliant thanks very much

emweb pushed a commit that referenced this pull request Apr 14, 2017
 - Added support for receiving and sending HTTP PATCH requests
   Original patch provided by user jamal-fuma on GitHub (PR #88/PR #104)
 - Wt::Json::parse overload for Wt::Json::Array
 - implicitly create json::value out of json::object and json::array
 - sql_value_traits for Wt::Json::Object and Wt::Json::Array, issue
   #5593
 - implemented HMAC function
 - resolveRelativeUrl:
   - handle .?, .# and .; correctly
   - Revert collapsing of .? into ?, because that's not ok
   - If we're at /foo, .?a=b => /?a=b whereas ?a=b => /foo?a=b
   - Documentation improvements
   - empty string should resolve to application deployment path
   - Fixed handling of . in relative URLs, we were mistaken
 - bookmarkUrl: use . if deployed at path ending in /, or application name if not
   . is not correct when we're not deployed at a path ending in / (because
   it will go to the parent directory, instead of the current application)
 - #5646, base64 encode the hash
 - Clarify in WTestEnvironment what the applicationPath variable does
 - Workaround for reparenting issue on Firefox
 - WAxis: Initialize renderInterval_ so that valgrind doesn't complain
   In debug builds, it never accesses the uninitialized memory, but with a
   release build for some reason it did.
 - bcrypt hash function arguments corrected
 - Also enable C++11 features when using Visual Studio 2015 or later
emweb pushed a commit that referenced this pull request Apr 14, 2017
 - Fixed CMake on Windows issue (test is now test.wt)
 - Added support for receiving and sending HTTP PATCH requests
   Original patch provided by user jamal-fuma on GitHub (PR #88/PR #104)
 - Wt::Json::parse overload for Wt::Json::Array
 - implicitly create json::value out of json::object and json::array
 - sql_value_traits for Wt::Json::Object and Wt::Json::Array, issue
   #5593
 - implemented HMAC function
 - resolveRelativeUrl:
   - handle .?, .# and .; correctly
   - Revert collapsing of .? into ?, because that's not ok
   - If we're at /foo, .?a=b => /?a=b whereas ?a=b => /foo?a=b
   - Documentation improvements
   - empty string should resolve to application deployment path
   - Fixed handling of . in relative URLs, we were mistaken
 - bookmarkUrl: use . if deployed at path ending in /, or application name if not
   . is not correct when we're not deployed at a path ending in / (because
   it will go to the parent directory, instead of the current application)
 - #5646, base64 encode the hash
 - Clarify in WTestEnvironment what the applicationPath variable does
 - Workaround for reparenting issue on Firefox
 - WAxis: Initialize renderInterval_ so that valgrind doesn't complain
   In debug builds, it never accesses the uninitialized memory, but with a
   release build for some reason it did.
 - bcrypt hash function arguments corrected
 - PR #111: Add support for forward secrecy to wthttp (Rutger ter Borg)
 - PR #110: Alter cmake to use folders with MSVC solution (Casey McCandless)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants