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

Add support for WebDAV http methods #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0x7f
Copy link
Contributor

@0x7f 0x7f commented Sep 8, 2017

I just wanted to do a small WebDAV test based on httpp. Found out that the methods defined in the WebDAV http extension are not supported by httpp atm, so I added them. Don't know whether you want them in the code base in the first place?

For details about the methods see:
http://www.webdav.org/specs/rfc2518.html
https://en.wikipedia.org/wiki/WebDAV

@0x7f
Copy link
Contributor Author

0x7f commented Sep 8, 2017

Just had a look at node's http-parser and they do support even more methods. What do you think about adding all of them?

https://github.com/nodejs/http-parser/blob/feae95a3a69f111bc1897b9048d9acbc290992f9/http_parser.h#L93-L134

COPY,
MOVE,
LOCK,
UNLOCK,
};

std::string to_string(Method method);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add those new methods to the to_string and method_from functions

@@ -101,6 +101,30 @@ static bool parse_method(Iterator& it, Request& request)
return true;
}
}
else if (*it == 'R')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the stream based parser is deprecated and you'll add many more methods, maybe we can either delete it or simplify it ?

@@ -253,6 +253,13 @@ void Connection::configureRequest(HTTPP::HTTP::Method method)
case HTTPP::HTTP::Method::OPTIONS:
case HTTPP::HTTP::Method::TRACE:
case HTTPP::HTTP::Method::CONNECT:
case HTTPP::HTTP::Method::PROPFIND:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice that you thought of that :)

@@ -110,7 +110,7 @@ bool Parser::parse(const char* start,
const char *token_begin, *token_end;

%%{
method = ("GET" | "POST" | "HEAD" | "PUT" | "DELETE" | "OPTIONS" | "TRACE" | "CONNECT");
method = ("GET" | "POST" | "HEAD" | "PUT" | "DELETE" | "OPTIONS" | "TRACE" | "CONNECT" | "PROPFIND" | "PROPPATCH" | "MKCOL" | "COPY" | "MOVE" | "LOCK" | "UNLOCK");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you don't add the methods in the method_from function, while parsed, you won't get the proper enum in the Request

@daedric
Copy link
Owner

daedric commented Sep 9, 2017

I'm totally fine to have all those methods supported :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants