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

Revert back to using custom HTTP parser instead of Boost.Beast #6407

Merged
merged 5 commits into from
Oct 14, 2022

Conversation

SiarheiFedartsou
Copy link
Member

@SiarheiFedartsou SiarheiFedartsou commented Oct 14, 2022

Issue

Context: #6400 (comment)
closes #6400

Tasklist

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@SiarheiFedartsou SiarheiFedartsou changed the title Sf revert http parser Revert back to using custom HTTP parser instead of Boost.Beast Oct 14, 2022
CHANGELOG.md Outdated
- FIXED: Fix inefficient osrm-routed connection handling [#6113](https://github.com/Project-OSRM/osrm-backend/pull/6113)
- FIXED: Fix HTTP compression precedence [#6113](https://github.com/Project-OSRM/osrm-backend/pull/6113)
Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIR this change was made as a fix of regression after #6294, but @mjjbell please confirm.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, that's right.

@@ -55,4 +55,4 @@ module.exports = function () {
assert.equal(this.processError.process, binary);
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't remove "large request" test which I added recently - it will be useful to avoid regression as was introduced by Boost.Beast migration.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed.

}
}
else
RequestParser::RequestStatus result;
Copy link
Member Author

Choose a reason for hiding this comment

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

All changes are made reverting git commits related to Boost.Beast and subsequent fixes.

Copy link
Member

@mjjbell mjjbell left a comment

Choose a reason for hiding this comment

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

LGTM. Works on my tests now.

My 5K coordinate query is just the one in #6400 with the last coord repeated thousands of times, so it's completely nonsensical, not sure it's worth adding.

@lamaux would you be able to test this PR on your failing requests as an extra confirmation?

@lamaux
Copy link

lamaux commented Oct 14, 2022

@lamaux would you be able to test this PR on your failing requests as an extra confirmation?

Tested it and works as expected

@SiarheiFedartsou
Copy link
Member Author

Thanks @lamaux
Then merging it.

@SiarheiFedartsou SiarheiFedartsou merged commit 9d160a9 into master Oct 14, 2022
datendelphin added a commit to fossgis-routing-server/osrm-backend that referenced this pull request Dec 23, 2022
v5.27.1

  - Changes from 5.27.0
    - Misc:
      - FIXED: Revert back to using custom HTTP parser instead of Boost.Beast. [Project-OSRM#6407](Project-OSRM#6407)
      - FIXED: Fix bug with large HTTP requests leading to Bad Request in osrm-routed. [Project-OSRM#6403](Project-OSRM#6403)
    - Routing:
      - CHANGED: Add support for surface=metal,grass_paver,woodchips in bicyle profile. [Project-OSRM#6395](Project-OSRM#6395)
mattwigway pushed a commit to mattwigway/osrm-backend that referenced this pull request Jul 20, 2023
@DennisOSRM DennisOSRM deleted the sf-revert-http-parser branch May 3, 2024 12:29
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

Successfully merging this pull request may close these issues.

mid-size table requests to osrm-routed lead to HTTP error 400 / bad request
3 participants