Added proxy_handler, modified request#36
Added proxy_handler, modified request#36konradjniemiec-zz wants to merge 17 commits intoUCLA-CS130:masterfrom
Conversation
|
Proxy Request currently hang. Host header needs to be changed and HttpVersion is wrong. Need to be fixed in ProcessRequest before calling PerformRequest. |
|
Can anyone take a look at this? It still hangs and I have no idea why even though I hardcoded a correct request. It says "Reading" And then hangs on the read. |
|
@konradjniemiec Are you asking us to look at this? Or your team members? |
|
@sgervais21 my team members. If you want to build this branch and take a shot at it too it would be appreciated as well haha |
|
Hey, has there been any progress on the proxy handler code? This pull request has not been updated for almost a week now. |
|
@mjchen04 @valeedmalik We are waiting for the professors to respond to us, we are stuck on what we think is a bug in boost. |
|
Has been fixed now. Only need integration tests and dependency injection. |
|
Hey guys. Added some preliminary and nit picky style change requests. Can you also apply the same to other files that I didn't comment on yet? Also, is the bulk of the functionality finished now and ready for code review? |
|
@mjchen04 yes, just have unit tests I'm finishing now. |
|
@mjchen04 I also don't know what style changes you are talking about, so I can't really add them. |
|
@konradjniemiec and team members. Are you able to see the comments I added to files? |
|
@mjchen04 I don't think we are able to see your comments, where would we be able to see them? |
| #include "proxy_handler.h" | ||
| #include "config_parser.h" | ||
| #include <boost/asio.hpp> | ||
| RequestHandler::Status ProxyHandler::Init(const std::string& uri_prefix, |
There was a problem hiding this comment.
Space before Init declaration
There was a problem hiding this comment.
What do you mean here? Line after the include?
proxy_handler.cc
Outdated
| #include "config_parser.h" | ||
| #include <boost/asio.hpp> | ||
| RequestHandler::Status ProxyHandler::Init(const std::string& uri_prefix, | ||
| const NginxConfig& config) |
There was a problem hiding this comment.
Idk if this is just the way github displays or not. But can you try to line up const on line 5 to line 4
proxy_handler.cc
Outdated
| } | ||
|
|
||
| RequestHandler::Status ProxyHandler::HandleRequest(const Request& request, | ||
| Response* response) |
There was a problem hiding this comment.
Line up Response with const
| } | ||
|
|
||
| RequestHandler::Status ProxyHandler::PerformRequest(Request& request, | ||
| Response* response, |
There was a problem hiding this comment.
Same line up of parameters
proxy_handler.cc
Outdated
| const std::string& host_port) { | ||
| try { | ||
| boost::system::error_code ec; | ||
| boost::asio::io_service io_service; |
| echo "Sending requests" | ||
|
|
||
| # Send request to server | ||
| curl -s www.ucla.edu:80 > temp_expected |
There was a problem hiding this comment.
Same for temp_expected and temp_response as temp_config
response.cc
Outdated
|
|
||
| std::string Response::ToString() { | ||
| std::string response_msg = "HTTP/1.1 " + to_string(status); | ||
| std::string response_msg = version+" "+ status; |
There was a problem hiding this comment.
Include spaces between operands and operators
response.h
Outdated
| bad_request = 400, | ||
| not_found = 404 | ||
| }; | ||
| static std::unique_ptr<Response> Parse(const std::string& raw_response) |
There was a problem hiding this comment.
can we move the implementation to .cc
proxy_handler.cc
Outdated
| boost::system::error_code ec; | ||
| boost::asio::io_service io_service; | ||
| boost::asio::ip::tcp::resolver resolver(io_service); | ||
| boost::asio::ip::tcp::resolver::iterator endpoint_iterator = resolver.resolve({host_uri,host_port},ec); |
There was a problem hiding this comment.
Remove extra space between iterator and endpoint..
| @@ -0,0 +1,58 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Is this doing the same thing as proxy_redirect.sh?
There was a problem hiding this comment.
changed so that they do different things
|
See if you can see them now. |
|
@mjchen04 Got most of them, put my refutation for others, sorry for being late with it was sick all weekend. Let me know if there is anything else. |
5 participants
Starting to implement proxy_handler, added skeleton code. Still need to be able to parse response. Will continue proxy_handler on this branch.