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

Send points to backend thru post instead of get #4211

Closed
diegocando opened this issue Jun 28, 2017 · 12 comments
Closed

Send points to backend thru post instead of get #4211

diegocando opened this issue Jun 28, 2017 · 12 comments

Comments

@diegocando
Copy link

Hi, I'm using OSRM 5.3.0 and doing some modifications because the amount of points sent thru uri are way too long and the connection is dropped, I've tried to implement this by myself but failed.

I changed the connection.cpp file to substring incomming_data_buffer, but seems that async_read_some trimms the content of the call.

can anybody tell me how do you implemented this? any lead?

I've also tried with async_recieve -> the connection isnt dropped but the received content is also trimmed

I've changed connection.hpp to change from 8k to 2m the size of incomming_data_buffer but still trimm the content receieved.

Also tried to use boost::asio::async_read but something is missing, the connection doesnt finishes besides transfered all the content (checked using sniffer)

@daniel-j-h
Copy link
Member

We removed POST support a while ago: #2182

For your use-case what could make more sense is to

  • either use the Node.js bindings and build your logic on top of it or
  • use the C++ libosrm library and build your logic on top of it

instead of bringing POST support back to osrm-routed.

https://github.com/Project-OSRM/osrm-backend#using-the-nodejs-bindings
https://github.com/Project-OSRM/osrm-backend/blob/master/docs/libosrm.md

@danpat
Copy link
Member

danpat commented Jun 28, 2017

Hmm. While incoming_data_buffer governs the amount of data that is handled per-read, the stream is not limited - requests can be much larger than incoming_data_buffer, osrm-routed has no limit AFAIK.

@diegocando how are you making requests? Are you sure it isn't your client library that is creating the limit?

From memory, I've tested large command-line requests with wget -i url_file where url_file contains the full URL on one line. The reason to read the URL from a file is because there are command-line limits (getconf ARG_MAX, typically 262kb on Linux, 32kb on Windows).

@diegocando
Copy link
Author

Hi Daniel, Dan, thank you for your answers.

@danpat , it's a good idea, but in the client side I'm using c# web so the request still is limited to IIS, I'll try to make some workaround to test your way that is a good one.

@daniel-j-h , I have a question, why POST was discarded? it seems like a smaller effort to implement post in the current project than changing to node.js or liborsm (the change will be a way better solution but will take some time extra, this will be the 2nd phase of the project but a bit later) but using async_read_some gets a partial from the request, but according to boost documentation async_read suits the case, but I'm stuck trying to implement this.

this is what I've done in connection.cpp

void Connection::start()
{
// TCP_socket.async_read_some(
boost::asio::async_read(TCP_socket(boost::asio::io_service),
// TCP_socket.async_receive(

    boost::asio::buffer(incoming_data_buffer),
    strand.wrap(boost::bind(&Connection::handle_read,

                            this->shared_from_this(),
                            boost::asio::placeholders::error,
                            boost::asio::placeholders::bytes_transferred)));

}

void Connection::handle_read(const boost::system::error_code &error, std::size_t bytes_transfer$
{
if (error)
{
return;
}

// no error detected, let's parse the request
http::compression_type compression_type(http::no_compression);
RequestParser::RequestStatus result;
std::tie(result, compression_type) =
    request_parser.parse(current_request,
                         incoming_data_buffer.data(),
                         incoming_data_buffer.data() + bytes_transferred);

// the request has been parsed
if (result == RequestParser::RequestStatus::valid)
{

std::string aux = incoming_data_buffer.data();
std::string aux1 = aux.substr(0,bytes_transferred);

if (current_request.uri.find("table") > 0 && current_request.uri.length() < 20)
//if th e points are in the body
current_request.uri =
current_request.uri+aux1.substr(aux1.find("points")+7,
aux1.length()-aux1.find("points"));
aux1.clear();

@danpat
Copy link
Member

danpat commented Jun 28, 2017

so the request still is limited to IIS,

@diegocando Are you accessing osrm-routed directly, or do you have it behind an IIS proxy?

If you're using the node bindings, then connection.cpp does not apply at all.

@diegocando
Copy link
Author

diegocando commented Jun 28, 2017

@danpat osrm-routed have its own server using the regular osrm-routed server, the iis is in the same network but on a different server.

@danpat
Copy link
Member

danpat commented Jun 28, 2017

@diegocando I just re-tested osrm-routed - there is no limit to the size of the GET request, you do not need to modify connection.cpp to change buffer sizes or boost async methods - you're looking in the wrong place for the problem.

I created a 4.2MB GET request by simply repeating coordinates:

$ head -c 150 request.txt
http://localhost:5000/route/v1/driving/7.413540,43.729615;7.436285,43.748963;7.413540,43.729615;7.436285,43.748963;7.413540,43.729615;7.436285,43.7489
$ wc -c request.txt
 4240839 request.txt

I used wget to fetch it:

$ wget -q -i request.txt -S -O -
  HTTP/1.0 200 OK
  Connection: close
  Access-Control-Allow-Origin: *
  Access-Control-Allow-Methods: GET
  Access-Control-Allow-Headers: X-Requested-With, Content-Type
  Content-Type: application/json; charset=UTF-8
  Content-Disposition: inline; filename="response.json"
  Content-Length: 66960027
{"waypoints":[{"location":[7.413513,43.729622],"hint...........<trimmed>

(this request took about 2 minutes to respond, and returned 70MB of data).

To support requests of this size, you need to run osrm-routed --max-viaroute-size 1000000 yourmap.osrm

If you're getting connection drops from osrm-routed, verify that you've increased the --max-viaroute-size from the default (500 coords), and that the problem doesn't lie within your client HTTP library.

@danpat
Copy link
Member

danpat commented Jun 28, 2017

For completeness: this block of code in connection.cpp shows how osrm-routed continues to read buffers from the network until it builds a valid request:

https://github.com/Project-OSRM/osrm-backend/blob/master/src/server/connection.cpp#L104

the size of a single HTTP request is not limited to the size of the read buffer. The handle_read method continues to add buffers to the result object until it determines that it's either complete and valid, or complete and invalid.

@daniel-j-h
Copy link
Member

Closing as not actionable on our side.

@diegocando
Copy link
Author

diegocando commented Jul 10, 2017

Hi guys, using the URI has a limit within who invokes the call, wget have no limits but another client does(iis, tomcat, etc), I'm sharing the solution I get to partialy implement a post request in the osrm-routed service, this might enable a better support for future versions.

The files I modified was:
connection.hpp (added another buffer to partially read the header of the message)
connection.cpp (copied 2 times the function handle_read to recieve the first part of the message, then do the old read or the new read when the message have body)
request_parser.cpp (so in the first part of connection.cpp i get the first 4 bytes of the message so in the "get" case the rest of the message must be parsed starting with de URI part)

Hope this help someone that might be seeking the same as me.

modified.zip

@RichardVogelij
Copy link

This seems like a bad design choice - I'm using c# HttpClient to interact with the OSRM API. I'm running into maximum URL length issues which would not exist if a POST were possible.

Simply forcing users to use some http client library (nodejs's) which is for all practicality the -only- http client not imposing such a limit is of course an answer, but that requires consumers of the API to proxy the request through a nodejs layer. Also any kind of load balancing is greatly more complex.

The entire world uses post to send lots of data to a server. Get was never intended for this use. If anything i'd advocate for removing the GET path if it is too much of a hassle to maintain both GET and POSTs

@danpat
Copy link
Member

danpat commented Jun 1, 2020

@RichardVogelij I'd class it less as a "bad design choice" and more of a "nobody has needed this feature enough to spend time to implement it yet". Hint hint.

I assume the limit you're hitting is this one?

https://docs.microsoft.com/en-us/dotnet/api/system.uri.-ctor?view=netcore-3.1#System_Uri__ctor_System_String_

The length of uriString exceeds 65519 characters.

One thing that might help temporarily is to control the precision of your coordinates - I often see requests with 10+ decimal places for each coordinate, which is totally unnecessary. If you're doing that, reducing it to 5 or 6 can save you some chars in the URL.

@RichardVogelij
Copy link

RichardVogelij commented Jun 1, 2020

@RichardVogelij I'd class it less as a "bad design choice" and more of a "nobody has needed this feature enough to spend time to implement it yet". Hint hint.

I assume the limit you're hitting is this one?

https://docs.microsoft.com/en-us/dotnet/api/system.uri.-ctor?view=netcore-3.1#System_Uri__ctor_System_String_

The length of uriString exceeds 65519 characters.

One thing that might help temporarily is to control the precision of your coordinates - I often see requests with 10+ decimal places for each coordinate, which is totally unnecessary. If you're doing that, reducing it to 5 or 6 can save you some chars in the URL.

Hi, thanks for your reply - Yes - i was refering to this limit indeed - In all honesty this only started to happen when I started experimenting with adding hints from previously obtained "nearest" requests in an attempt to find out if I was not missing out on a possible performance boost. These hints are quite significant b64 strings, and when I add those i reach that threshold of 65.5K characters in no time when generating a table/weight matrix.

For the record - adding that hint data to my table request did not seem to give any difference - So I've stopped adding this data anyway - I'm well below the 65K threshold again.

I was just -really- surprised to find: #2163 and felt this is a step in the wrong direction.

OSRM is however an awesome project by the way!

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

No branches or pull requests

4 participants