Merged
Conversation
According to CURL source that old macro is long gone, and was replaced. Real issue is: you don't find the old macro in CURL's documentation anymore, which makes it harder to understand what libhttpserver is doing here. Signed-off-by: Alexander Dahl <ada@thorsis.com>
Preparation for adding an interface to get a list of only allowed methods aka methods where the boolean property is true. References: etr#240 Suggested-by: Sebastiano Merlino <sebastiano@hey.com> Signed-off-by: Alexander Dahl <ada@thorsis.com>
While the class had public methods for setting and testing the currently allowed HTTP methods, there was no way to discover all at once. A hypothetic user could have build such a list by calling 'is_allowed()' on each possible HTTP method, but the http_resource class itself contains the actual comprehensive list (map) of methods, which would make that approach error prone. Such a list is needed however for constructing the HTTP header 'Allow:' which is mandatory under certain conditions according to RFC. References: etr#240 Signed-off-by: Alexander Dahl <ada@thorsis.com>
LeSpocky
commented
Nov 12, 2021
Contributor
Author
|
The other thing cpplint complains about is header ordering, which will be easy to fix. |
We want to test the recently introduced new API methods. References: etr#240 Signed-off-by: Alexander Dahl <ada@thorsis.com>
RFC 7231 states an additional header "Allow:" must be sent along with a response with code 405 (Method Not Allowed). References: etr#240 Link: https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.5 Link: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow Signed-off-by: Alexander Dahl <ada@thorsis.com>
LeSpocky
force-pushed
the
method-not-allowed
branch
from
0f12fa8 to
e4def6d
Compare
Contributor
Author
Done that and force pushed a new version. Lets see what CI says now?! |
etr
reviewed
Nov 12, 2021
Owner
etr
left a comment
etr
left a comment
There was a problem hiding this comment.
It all looks good, only one comment to use the constant defined in http_utils for the "Allow" header.
RFC 7231 states an additional header "Allow:" MUST be sent along with a response with code 405 (Method Not Allowed). Fixes: etr#240 Link: https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.5 Link: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow Signed-off-by: Alexander Dahl <ada@thorsis.com>
LeSpocky
force-pushed
the
method-not-allowed
branch
from
e4def6d to
538c560
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Identify the Bug
#240
Description of the Change
To be able to send along an additional header like
Allow: GET, POSTwith an HTTP 405 (Method Not Allowed) response, thehttpserver::http_resourceclass had to be extended to expose the allowed methods already stored in that object. Those can be used in the webserver then to build the necessary header line and attach it to the response.Alternate Designs
The proposed version was selected based on discussion in the bug report #240.
Possible Drawbacks
httpserver::http_resourcehad to be changed)Verification Process
Tests were added:
httpserver::http_resourceRelease Notes
http_resourcenow allows to query the currently set allowed methods