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.

Sign up for GitHub

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

Fix bug with large HTTP requests leading to Bad Request in osrm-routed. #6403

Merged
merged 4 commits into from
Oct 13, 2022
Merged

Fix bug with large HTTP requests leading to Bad Request in osrm-routed. #6403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce49810
Remove upper limit for HTTP headers size
SiarheiFedartsou Oct 12, 2022
16b8a58
Fix bug with large HTTP requests in osrm-routed
SiarheiFedartsou Oct 12, 2022
66b7386
Fix bug with large HTTP requests in osrm-routed
SiarheiFedartsou Oct 12, 2022
851b8ec
Fix bug with large HTTP requests in osrm-routed
SiarheiFedartsou Oct 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased
- Changes from 5.27.0
- Misc:
- FIXED: Fix bug with large HTTP requests leading to Bad Request in osrm-routed. [#6403](https://github.com/Project-OSRM/osrm-backend/pull/6403)
- Routing:
- CHANGED: Add support for surface=metal,grass_paver,woodchips in bicyle profile. [#6395](https://github.com/Project-OSRM/osrm-backend/pull/6395)

Expand Down
6 changes: 3 additions & 3 deletions features/step_definitions/requests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module.exports = function () {
this.When(/^I request \/(.*)$/, (path, callback) => {
this.reprocessAndLoadData((e) => {
if (e) return callback(e);
Copy link
Member Author

Choose a reason for hiding this comment

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

This was unused and probably contained obsolete assumptions because of it.

this.requestPath(path, {}, (err, res, body) => {
this.requestUrl(path, (err, res, body) => {
this.response = res;
callback(err, res, body);
});
Expand All @@ -23,13 +23,13 @@ module.exports = function () {
this.ShouldBeWellFormed();
});

this.Then(/^status code should be (\d+)$/, (code, callback) => {
this.Then(/^status code should be (.+)$/, (code, callback) => {
try {
this.json = JSON.parse(this.response.body);
} catch(e) {
return callback(e);
}
assert.equal(this.json.status, parseInt(code));
assert.equal(this.json.code, code);
callback();
});

Expand Down
39 changes: 39 additions & 0 deletions features/stress/large_request.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Feature: Large requests
Background:
Given the profile "testbot"
Scenario: Table request with a lot of input coordinates
Given the origin 10.90761,47.54966
Given the node map
"""
a b c d e f g h i j k l m n o p q r s t u v w x y z
"""

And the ways
| nodes |
| ab |
| bc |
| cd |
| de |
| ef |
| fg |
| gh |
| hi |
| ij |
| jk |
| kl |
| lm |
| mn |
| no |
| op |
| pq |
| qr |
| rs |
| st |
| tu |
| uv |
| vw |
| wx |
| xy |
| yz |
When I request /table/v1/driving/10.90761,47.54966;11.93966,47.81606;11.08894,48.62424;11.62135,47.48357;11.5487,47.49729;11.45756,48.03065;11.23334,47.89373;10.88259,47.54795;11.4702,49.52285;11.62638,48.46008;11.98631,49.03973;11.92384,48.65348;11.52325,47.88653;11.56097,48.51048;11.08578,48.77129;11.96313,49.79663;11.7655,49.68107;10.74547,48.18696;11.35802,47.92084;11.31547,47.75373;11.29578,47.63577;11.22657,48.50509;11.31266,48.20597;11.11292,49.30903;10.91056,48.85395;11.01956,47.91374;10.63385,47.91534;11.37957,48.89772;11.23514,47.60342;11.00552,49.66307;11.09602,49.06538;10.58208,49.7129;11.08048,47.95764;10.66334,49.54911;11.99811,47.51836;11.06729,48.93516;11.51662,47.72612;11.3375,49.24294;11.90311,48.68334;11.92122,48.35409;10.87577,49.49948;11.21114,49.42086;10.87195,47.84828;10.98919,49.75728;10.6987,49.77116;11.97602,49.18479;11.94726,48.55827;10.87772,48.65176;10.52576,49.85704;10.6882,47.7531;11.05737,48.99173;11.85929,48.19158;10.84597,49.55252;11.32428,49.21267;11.3375,48.10871;11.01082,48.01289;10.7295,47.94004;11.23957,47.46262;11.80514,48.69129;11.33402,48.81084;11.42753,49.18682;10.86449,48.93515;11.28717,47.7437;11.25911,47.73088;11.18528,48.51873;11.47802,48.44734;11.84338,49.19166;11.23232,49.73384;11.73743,49.92189;10.81741,48.85105;11.96345,49.52129;11.19351,48.71871;11.64791,47.78599;11.56445,48.27622;10.86725,47.86015;10.9428,48.51327;10.84549,48.9471;11.09816,49.28497;11.33345,47.56826;11.06624,47.50943;10.83086,49.66004;10.99761,48.44961;11.1311,49.73714;11.22288,47.52466;11.57479,49.61386;10.55757,47.50472;11.35294,49.75207;11.62176,49.48058;10.69253,49.30151;11.74453,49.66988;10.73915,49.06721;10.66147,47.60705;11.96606,47.62438;11.90213,47.5219;11.23404,47.52766;10.7231,47.87407;11.84783,48.3516;11.84069,48.9192;11.58223,48.65717;10.61954,49.0955;11.88673,49.56121;11.07369,48.32443;11.96949,48.52898;11.30511,48.88208;11.87258,47.91028;11.73445,47.52629;11.63164,49.20231;10.793,49.60652;11.24185,49.92677;10.98617,49.14844;11.01802,49.40513;11.05101,48.06117;11.31047,47.69413;11.07766,49.93023;10.65685,49.11901;10.60773,49.41996;10.95122,48.79153;11.4117,49.4951;11.69883,49.37566;11.82451,49.80235;11.89671,48.40329;11.11972,48.73538;10.57045,49.41698;10.74612,49.68198;11.7765,48.09569;11.04161,49.8671;10.95917,49.12747;10.91793,47.97454;10.54507,48.82384;10.57876,48.75478;10.76263,49.23102;11.33573,47.86198;11.17414,47.88678;11.65555,49.99393;10.77196,49.83992;11.31043,48.56057;10.94094,49.59839;11.92389,49.34819;11.48685,47.73434;11.46912,48.12031;11.34077,49.13553;11.00964,48.8761;11.44085,47.5319;10.52021,49.26626;11.9019,47.65143;11.51561,49.35583;10.52268,48.85212;10.86347,48.31117;11.25467,49.7146;10.52205,48.27031;11.60986,49.48577;11.68148,49.72743;11.75741,49.81563;11.05122,49.80071;11.94883,49.63083;11.02932,48.14442;11.78529,47.96527;11.23927,49.0946;11.18084,48.08152;10.53525,48.98016;10.80783,47.72982;10.81771,47.8415;11.88089,48.06763;10.75608,49.00451;11.09311,48.08249;10.70716,49.44809;10.78888,48.51296;10.95841,49.54797;10.88098,47.45088;10.98406,49.38162;11.51657,48.7231;11.15303,49.65487;10.72106,47.63585;11.87195,48.92029;11.96055,48.30037;11.86625,48.04542;10.74646,48.1694;11.29419,49.29692;11.77349,48.38297;11.61929,49.81509;10.50075,48.2405;11.60097,47.89965;11.68649,48.05119;11.81297,49.76957;10.95531,47.60711;11.99713,48.16887;11.93346,48.57039;11.47905,49.24811;11.10927,49.67728;10.6618,48.28644;11.54246,47.59938;11.51245,49.08808;10.61251,47.46968;10.85949,48.89206;11.05734,47.81972;11.46352,47.90034;11.86184,49.53277;10.95108,49.1559;10.90376,47.7312;11.13149,48.87966;11.84491,49.03545;11.33474,48.39079;10.84499,48.69141;10.67208,49.11536;11.1539,49.54286;10.64563,48.82016;11.53696,48.23053;11.98903,49.80495;10.72386,47.45824;11.23699,48.72638;11.63742,49.29705;11.5438,49.42475;10.78755,47.52848;11.7802,47.56707;11.4288,49.54467;10.74564,49.96728;11.20549,49.71876;11.65907,48.62485;11.39492,49.18665;11.09157,48.06504;10.99312,49.61296;11.89019,48.55865;10.8119,48.04861;11.36204,48.5806;10.91501,49.12398;11.17399,48.48726;11.04085,47.92358;11.06903,48.0575;10.73976,48.30711;11.93584,49.70889;10.5968,47.72353;11.52694,49.55474;10.81782,49.40075;11.84217,48.03225;11.22768,47.98959;10.6082,49.22826;11.46862,49.02127;11.33829,48.41548;11.71223,49.19104;11.41975,48.94869;11.61803,48.96822;10.50347,47.48205;11.60651,49.06259;11.35187,47.93713;10.76661,49.42307;11.86695,49.74779;10.83986,48.57656;10.551,48.90838;11.76523,47.57251;10.55009,49.54098;11.88052,49.85604;11.83511,48.13701;11.22141,49.58347;11.56253,49.80543;11.78782,49.46533;11.57083,49.1185;10.76972,47.67795;10.88535,48.09409;11.93944,48.10515;11.67993,49.27536;11.5906,48.76538;10.63163,48.23638;11.88353,48.85089;11.7295,49.08962;10.63416,49.89273;10.72741,49.7831;11.01236,49.67462;11.04184,49.25215;11.95828,47.93858;11.22746,49.47516;11.97998,49.69944;11.75393,48.84457;11.7618,47.4792;11.51812,48.29168;11.71144,47.66958;11.6494,47.89514;11.84211,49.2127;11.34524,48.6433;10.77714,47.89613;11.40105,47.87601;11.60605,47.89686;10.88092,48.80947;11.97652,49.47796;11.33343,48.80368;11.79732,48.53003;11.55873,49.8888;11.05606,47.59527;11.93572,48.82335;11.65693,49.40953;11.78328,48.46995;11.39285,49.37936;10.84134,47.63308;11.95604,48.11911;11.18343,47.93176;11.95007,48.46438;11.50128,49.25897;11.70257,48.32708;11.79472,49.3613;11.96752,47.99815;11.69199,49.0501;11.12854,49.49936;10.78246,49.26896;11.60707,48.32754;10.65964,49.35687;11.44447,48.32026;11.60625,49.76655;10.74575,49.28163;10.89368,47.58725;11.90005,48.35195;11.95757,48.26339;11.89691,49.36058;10.50237,47.45156;11.29258,47.56293;11.26933,48.4134;11.25534,49.5038;10.92052,49.53341;11.63137,49.01404;11.13264,49.12652;11.30996,48.12275;11.3125,49.31107;11.18353,48.68894;10.52496,48.60723;11.5545,48.01744;11.28575,49.18151;10.60623,47.51181;11.82353,48.54418;10.51634,48.6607;10.54879,48.65214;11.90644,49.8917;11.32017,49.96297;11.01123,49.2065;10.50854,49.59417;10.95013,47.6372;10.73682,49.61629;11.64592,49.94222;11.45368,48.44183;10.64348,48.73343;11.57665,48.77333;11.66549,48.91217;10.71938,47.49757;11.12204,47.53107;10.91103,49.56696;11.95834,47.61315;10.61409,49.36993;11.10758,48.16737;11.19082,49.90288;11.44083,48.99006;11.13477,48.51637;10.95816,48.25012;10.55454,49.99526;10.76371,49.07832;11.60491,48.7684;11.14329,47.5639;11.76087,48.28423;11.06633,48.71908;10.63479,49.07467;11.11831,47.63672;10.51461,49.97606;11.79856,47.51325;11.60394,47.54465;11.78364,49.20929;10.99021,47.92171;10.54795,49.69804;11.9299,49.29828;10.74526,49.44884;10.79157,48.52783;10.73713,49.77397;11.61066,48.41798;11.24592,49.46019;11.33682,49.80602;11.18163,49.94106;10.99738,49.86443;11.29373,47.8579;10.71569,47.74042;10.66168,48.85985;10.88105,49.04741;11.95581,49.23316;11.15938,49.25041;11.75631,47.92095;11.66879,49.83568;11.19202,47.87232;11.26559,49.71521;11.28089,49.76687;10.68274,47.54061;10.6705,48.7493;11.11543,49.90564;11.31847,47.9317;10.98724,49.44027;10.58021,49.00669;10.65988,47.61622;11.74486,48.46057;10.53387,48.56251;11.87333,48.81495;11.86938,48.84568;11.17144,49.90105;11.96628,48.80487;11.3636,47.6345;10.80856,48.00151;11.95887,48.42017;10.84812,49.93127;11.57792,49.40493;11.24702,48.58841;10.53697,49.99427;11.55126,47.45923;11.76386,49.58956;10.72076,48.66803;11.69896,49.06466;10.87897,48.73902;11.7625,48.90216;11.41714,49.13881;10.86463,47.94817;10.84486,49.10723;11.32144,48.12671;11.72769,47.69392;10.95778,48.65459;10.7602,48.44322;11.32139,49.3135;10.75378,47.71793;11.75266,49.04782;10.64936,49.16888;11.02814,47.82052;11.67243,49.14524;10.50856,48.8563;11.35956,48.54955;10.51606,49.55741;11.73378,48.37489;11.71308,49.39755;11.36699,47.45139;10.53954,47.96051;10.8803,49.53059;11.97025,49.07966;10.71181,48.53344;11.48929,47.88551;10.78747,49.25704;11.10914,49.89777;11.66374,48.79059;11.69725,47.89964;10.65969,49.30469;11.01374,48.55548;11.85932,47.67318;11.77681,48.82834;11.91272,48.76547;11.57922,49.2744;10.52352,47.58819;11.44351,49.98144;10.8438,48.29292;11.0613,49.23531;11.75374,47.77367;10.68599,48.88375;10.8381,48.89306;11.78983,48.57816;10.86193,47.86047;11.57646,48.46316;11.6558,48.6255;11.31754,48.56296;10.69917,47.52159;11.48864,47.93436;11.14443,49.34194;10.66421,48.15455;11.69012,49.71597;10.92805,49.39795;10.98026,48.62985;10.63252,49.95795;11.37468,48.32383;11.89213,49.41892;11.39476,48.36131;10.61696,47.97164;11.50761,48.21042;11.31284,48.29539;10.87814,48.67741;10.59776,49.1498;11.17827,48.8733;11.6018,49.45914;10.73621,49.5317;11.38794,49.0252;11.40146,49.55482;10.57116,48.86615;11.89119,48.7372;11.33492,47.66424;11.61472,49.9513;11.63065,48.67025;11.15378,48.6575;11.31485,47.94025;10.55891,47.48489;11.80567,49.04565;10.74453,49.99566;10.74512,47.61681;10.91615,48.93038;11.03106,49.41157;11.66623,48.85904;10.63791,47.50966;11.38506,47.85158;11.82981,48.99957;11.20183,48.28544;11.53728,49.93828;10.84794,49.48396;11.99693,48.55948;11.37453,48.06193;10.79992,48.12021;11.85343,48.03751?generate_hints=false&sources=0&destinations=all&skip_waypoints=true
Copy link
Member Author

Choose a reason for hiding this comment

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

Request from original issue

Then status code should be Ok
2 changes: 1 addition & 1 deletion include/server/connection.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Connection : public std::enable_shared_from_this<Connection>
boost::asio::deadline_timer timer;
RequestHandler &request_handler;
std::optional<RequestParser> http_request_parser;
boost::array<char, 8192> incoming_data_buffer;
std::vector<char> incoming_data_buffer;
http::request current_request;
http::reply current_reply;
std::vector<char> compressed_output;
Expand Down
27 changes: 19 additions & 8 deletions src/server/connection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@ namespace osrm
namespace server
{

namespace
{
const size_t CHUNK_SIZE = 8192;

} // namespace
Connection::Connection(boost::asio::io_context &io_context, RequestHandler &handler)
: strand(boost::asio::make_strand(io_context)), TCP_socket(strand), timer(strand),
request_handler(handler), http_request_parser(std::make_optional<RequestParser>())
{
http_request_parser->header_limit(std::numeric_limits<std::uint32_t>::max());
}

boost::asio::ip::tcp::socket &Connection::socket() { return TCP_socket; }
Expand Down Expand Up @@ -59,7 +65,8 @@ void Connection::start()
}
}

void Connection::handle_read(const boost::system::error_code &error, std::size_t bytes_transferred)
void Connection::handle_read(const boost::system::error_code &error,
std::size_t /*bytes_transferred*/)
{
if (error)
{
Expand All @@ -79,20 +86,23 @@ void Connection::handle_read(const boost::system::error_code &error, std::size_t
}

boost::beast::error_code ec;
http_request_parser->put(boost::asio::buffer(incoming_data_buffer, bytes_transferred), ec);
http_request_parser->put(boost::asio::buffer(incoming_data_buffer), ec);
Copy link
Member Author

Choose a reason for hiding this comment

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

We used it wrong for the case when request size > 8192 too... 🤦🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

https://www.boost.org/doc/libs/develop/libs/beast/doc/html/beast/ref/boost__beast__http__parser/put.html

This function attempts to incrementally parse the HTTP message data stored in the caller provided buffers. Upon success, a positive return value indicates that the parser made forward progress, consuming that number of bytes. In some cases there may be an insufficient number of octets in the input buffer in order to make forward progress. This is indicated by the code error::need_more. When this happens, the caller should place additional bytes into the buffer sequence and call put again. The error code error::need_more is special. When this error is returned, a subsequent call to put may succeed if the buffers have been updated. Otherwise, upon error the parser may not be restarted.

// no error detected, let's parse the request
http::compression_type compression_type(http::no_compression);

if (ec)
{
if (ec == boost::beast::http::error::need_more)
{
const auto current_size = incoming_data_buffer.size();
incoming_data_buffer.resize(incoming_data_buffer.size() + CHUNK_SIZE, 0);
// we don't have a result yet, so continue reading
TCP_socket.async_read_some(boost::asio::buffer(incoming_data_buffer),
boost::bind(&Connection::handle_read,
this->shared_from_this(),
boost::asio::placeholders::error,
boost::asio::placeholders::bytes_transferred));
TCP_socket.async_read_some(
boost::asio::buffer(incoming_data_buffer.data() + current_size, CHUNK_SIZE),
boost::bind(&Connection::handle_read,
this->shared_from_this(),
boost::asio::placeholders::error,
boost::asio::placeholders::bytes_transferred));
}
else
{
Expand Down Expand Up @@ -183,7 +193,8 @@ void Connection::handle_write(const boost::system::error_code &error)
current_request = http::request();
current_reply = http::reply();
http_request_parser.emplace();
incoming_data_buffer = boost::array<char, 8192>();
http_request_parser->header_limit(std::numeric_limits<std::uint32_t>::max());
incoming_data_buffer.resize(CHUNK_SIZE, 0);
output_buffer.clear();
this->start();
}
Expand Down