-
Notifications
You must be signed in to change notification settings - Fork 534
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
Initial work #1
Initial work #1
Conversation
@pmlopes Can you review this please? |
* @author <a href="http://tfox.org">Tim Fox</a> | ||
*/ | ||
@VertxGen | ||
public interface Bodies extends Handler<RoutingContext> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am seeing this and am remembering some comments from Yoke users that we probably should split Multipart parsing and POST body parsing. The reason is that for APIs you might want to parse the HTTP body usually with a JSON/XML payload, but allowing multipart is not desired since it could lead to DoS with big files uploads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added bodyLimit functionality which should work even if multipart.
if (groups != null) { | ||
// Pattern - named params | ||
for (String param : groups) { | ||
params.put(param, m.group(param)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not planning to support param like in Yoke?
https://github.com/pmlopes/yoke/blob/develop/framework/src/test/java/com/jetdrone/vertx/yoke/test/middleware/Router.java#L137
if you have a route /users/:id you can add a param('id', regex) or param('id', handler) and it will be called before the handler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Didn't know Yoke had that functionality. Will take a look :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add param to todo list for apex is it looks like useful functionality
Paulo - I've updated the PR to remove the system.out.println. The other issues I'm going to add a GitHub issues after this is merged to master. Then I'm going to create new GitHub issues for each of the remaining apex tasks/issues and we can assign to them to ourselves. Then we can both submit PRs against master when we do them. How does that sound to you? |
Improvement: On socket_idle, close socket only is event is completed.
No description provided.