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

Fix: Query parameter including query delimiter ('?') not being parsed properly #1713

Conversation

davidalo
Copy link
Contributor

When query delimiter ? is contained inside the query parameters, split function is cutting it and query parameters and not being parsed properly.

As an example:

/regex-with-delimiter?key=^(?.*(value))

This PR fixes it by adding m (maximum) parameter to details::split. This parameter indicates the maximum number of parts which will be got when splitting: -1 (infinite, no max), >= 0.

In Server::parse_request_line we use it as m=2 so we only get two parts: url and query parameters. By this way, we avoid splitting query parameters if they contain ?.

@yhirose
Copy link
Owner

yhirose commented Nov 17, 2023

@davidalo thank you very for your pull request. The change looks good to me. Could you make a couple of minor adjustments to make the code more readable?

  1. Change int for m to size_t
  2. Change -1 to std::numeric_limits<size_t>::max()
  3. Make the following helper function to pass std::numeric_limits<size_t>::max()
inline void split(const char *b, const char *e, char d,
                  std::function<void(const char *, const char *)> fn) {
  split(b, e, d, std::numeric_limits<size_t>::max(), fn);
}
  1. Change all the callers back to originals
  2. Change the line if (b[i] == d && (m < 0 || count < m)) { to if (b[i] == d && count < m) {

@yhirose yhirose added the bug label Nov 26, 2023
@davidalo
Copy link
Contributor Author

davidalo commented Dec 7, 2023

Sure, I will make those changes and notify you when available. Thanks for your reply.

@davidalo davidalo force-pushed the fix-query-parameter-including-query-delimiter-not-being-parsed-properly branch 3 times, most recently from 00a8c06 to bc28bc8 Compare December 7, 2023 08:23
@davidalo davidalo force-pushed the fix-query-parameter-including-query-delimiter-not-being-parsed-properly branch from bc28bc8 to 75914a9 Compare December 7, 2023 08:25
@davidalo
Copy link
Contributor Author

davidalo commented Dec 7, 2023

@yhirose I made those adjustments and I fixed conflicts. PR should be ready now.

@yhirose
Copy link
Owner

yhirose commented Dec 7, 2023

@davidalo thanks a lot for the fine contribution!

@yhirose yhirose merged commit e426a38 into yhirose:master Dec 7, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants