Skip to content

Adjusting rule chain logging; adjusting cookie header parsing #2203

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

Closed
wants to merge 2 commits into from
Closed

Conversation

martinhsv
Copy link
Contributor

No description provided.

@zimmerle zimmerle self-assigned this Nov 20, 2019
@zimmerle zimmerle added the 3.x Related to ModSecurity version 3.x label Nov 20, 2019
@zimmerle zimmerle added this to the v3.1.0 milestone Nov 20, 2019
@airween
Copy link
Member

airween commented Nov 21, 2019

Hi @martinhsv,

just my not-so-essential opinion, but the 2fb7eef commit has no effect, because in our last affected commit we remove the unnecessary spaces from begin of all cookie keys.

Instead of this new method, I think it would make more sense to modify the other trim method:

   559              while (!final_cookie_pair.empty() && isspace(final_cookie_pair.back())) {
   560                  final_cookie_pair.pop_back();
   561              }

by this:

   559              while (!final_cookie_pair.empty() && (isspace(final_cookie_pair.back()) || final_cookie_pair.back() == ';')) {
   560                  final_cookie_pair.pop_back();
   561              }

With this we can trim the cookie string like these:
Cookie: foo=bar; baz=foo // this works now
Cookie: foo=bar; baz=foo ; // this would be more useful
Cookie: foo=bar; baz=foo ; // this could gives back same
Cookie: foo=bar; baz=foo; // same here

What do you think?

@zimmerle
Copy link
Contributor

1b4dcdc is now merged and part of v3/master.

@zimmerle zimmerle self-requested a review November 21, 2019 11:33
@zimmerle
Copy link
Contributor

Merged last @martinhsv`s changes. Thank you.

@zimmerle zimmerle closed this Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants