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 usage of invalid variables #1187

Closed
wants to merge 3 commits into
base: v3.2/dev
from

Conversation

Projects
None yet
7 participants
@victorhora
Member

victorhora commented Sep 11, 2018

MATCHED_VAR_NAMES is an invalid variable with ModSecurity. It should be MATCHED_VARS_NAMES.

This should allow CRS 3.2/dev to load cleanly with the current version of libModSecurity.

Thanks! :)

@dune73

This comment has been minimized.

Collaborator

dune73 commented Sep 12, 2018

Well spotted. Thank you.

There are additional occurrences in lowercase letters. Would you mind fixing those as well?

@dune73

This comment has been minimized.

Collaborator

dune73 commented Sep 12, 2018

Wow, I was not aware the lowercase variable names were so widespread. Thanks.

Unfortunately, you have not fixed all the occurrences of MATCHED_VAR_NAMES. They are all in capital letters now, but a lot still miss the S.

@spartantri

This comment has been minimized.

Collaborator

spartantri commented Sep 12, 2018

wow, it's everywhere! old modsec never complained of our misspellings on this, so never really notice it until now

@victorhora

This comment has been minimized.

Member

victorhora commented Sep 12, 2018

@dune73 unless I screwed up something, both commits should result in no invalid variable names:

image

Maybe you just checked the latest commit?

@dune73

This comment has been minimized.

Collaborator

dune73 commented Sep 13, 2018

Sorry. My bad. I saw MATCHED_VAR_NAME in your commit (right before going to bed).

@dune73

This comment has been minimized.

Collaborator

dune73 commented Sep 13, 2018

Looks ready to be merged to me.

@dune73

dune73 approved these changes Sep 13, 2018

@victorhora victorhora changed the title from Fix usage of unexistent variable (MATCHED_VAR_NAMES) to Fix usage of invalid variables Sep 13, 2018

@fgsch

This comment has been minimized.

Collaborator

fgsch commented Sep 17, 2018

IMO the fix for MATCHED_VAR_NAMES should be done separately from the case changes and backported to 3.1.

The main issue in this PR is specific to 3.2. Sorry for the noise.

@fzipi

This comment has been minimized.

Collaborator

fzipi commented Nov 7, 2018

I managed to reapply this PR on top of 1214, in the new #1229 . Also, backported to v3.1/dev in #1233.

Sorry @victorhora for me being to eager in merging 1214, and thanks for this one.

@franbuehler

This comment has been minimized.

Collaborator

franbuehler commented Nov 8, 2018

Closing this PR without merging because of @fzipi PR #1229.
Thank you @victorhora for finding these variables and for writing this PR!

@franbuehler franbuehler closed this Nov 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment