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

Updated HTTPRouter.any function to handle any method request #984

Merged
merged 3 commits into from Feb 21, 2015

Conversation

Projects
None yet
3 participants
@gedaiu
Contributor

gedaiu commented Feb 14, 2015

No description provided.

@Geod24

This comment has been minimized.

Show comment
Hide comment
@Geod24

Geod24 Feb 15, 2015

Contributor

HEAD / OPTIONS / TRACE / CONNECT should definitively go in, but I don't think WebDAV method should be there by default. It would probably be better to provide another method (anyWebDAV ?) to explicitely set those.

Contributor

Geod24 commented Feb 15, 2015

HEAD / OPTIONS / TRACE / CONNECT should definitively go in, but I don't think WebDAV method should be there by default. It would probably be better to provide another method (anyWebDAV ?) to explicitely set those.

@gedaiu

This comment has been minimized.

Show comment
Hide comment
@gedaiu

gedaiu Feb 15, 2015

Contributor

Hi,

When I see ANY, I am expecting to have the routing to any method, even is some method that is not specified in documentation.

I am ok with adding another function for anyWebDAV, but if I add this method what we route there? Because webdav needs the GET / PUT / OPTIONS ... etc methods to work and someone who want to implement a webdav protocol might be fooled by "anyWebDAV" if it does not route the basic HTTP methods.

Maybe there should be: anyHttp -> that route all the basic http methods and any that routes all the requests.

what do you think?

Thanks,
Bogdan

Contributor

gedaiu commented Feb 15, 2015

Hi,

When I see ANY, I am expecting to have the routing to any method, even is some method that is not specified in documentation.

I am ok with adding another function for anyWebDAV, but if I add this method what we route there? Because webdav needs the GET / PUT / OPTIONS ... etc methods to work and someone who want to implement a webdav protocol might be fooled by "anyWebDAV" if it does not route the basic HTTP methods.

Maybe there should be: anyHttp -> that route all the basic http methods and any that routes all the requests.

what do you think?

Thanks,
Bogdan

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 15, 2015

Member

At least the semantics that were originally intended are to really match any request method. A typical use would be to intercept all requests with router.any("*", ...). If such an interception doesn't work for some methods that might easily introduce security issues or similar. I'd even go as far and change the implementation for foreach over all possible values of HTTPMethod to avoid missing anything in the future.

Member

s-ludwig commented Feb 15, 2015

At least the semantics that were originally intended are to really match any request method. A typical use would be to intercept all requests with router.any("*", ...). If such an interception doesn't work for some methods that might easily introduce security issues or similar. I'd even go as far and change the implementation for foreach over all possible values of HTTPMethod to avoid missing anything in the future.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Feb 15, 2015

Member

@Geod24 BTW, do you have a specific use case in mind for separate matching of usual and WebDAV specific methods?

Member

s-ludwig commented Feb 15, 2015

@Geod24 BTW, do you have a specific use case in mind for separate matching of usual and WebDAV specific methods?

s-ludwig added a commit that referenced this pull request Feb 21, 2015

Merge pull request #984 from gedaiu/master
Updated HTTPRouter.any function to handle any method request

@s-ludwig s-ludwig merged commit 3894447 into vibe-d:master Feb 21, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment