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

Rule to check if both C-L and T-E are present #1310

Closed
wants to merge 1 commit into
base: v3.2/dev
from

Conversation

Projects
None yet
2 participants
@fgsch
Copy link
Collaborator

fgsch commented Feb 18, 2019

SSIA.

@fgsch fgsch force-pushed the fgsch:cl_te-rule branch 2 times, most recently from f506b4c to 00f7f63 Feb 18, 2019

@fgsch fgsch force-pushed the fgsch:cl_te-rule branch from 00f7f63 to 649c3b5 Feb 18, 2019

@dune73

This comment has been minimized.

Copy link
Collaborator

dune73 commented Feb 18, 2019

Nice rule. Thank you. Two questions:

Strict sibling?

I think this is not really a strict sibling, so I think it should get it's proper independent rule id ending in 0.

Comment?

The test says:

              # Apache unsets the Content-Length header if
              # Transfer-Encoding is found!
              no_log_contains: id "920181"

Could you explain this and what it means for the rule?

@fgsch

This comment has been minimized.

Copy link
Collaborator Author

fgsch commented Feb 18, 2019

It says "related to" but I can remove the comment and have a separate id, though I wanted to keep them close since they are really related.

As for the test, apache silently removes the Content-Length header so the test fails. I think this is wrong in many accounts but that aside, the rules are not apache specific and the tests should cover them. By leaving the test enable we have the chance to find out how other consumers of these rules behave and whether we should update them.

@dune73

This comment has been minimized.

Copy link
Collaborator

dune73 commented Mar 4, 2019

Monthly chat: Apache blocks this from happening anyways and we're not sure what NGINX is doing. Maybe the rule is redundant on all webservers. So we'll put it on hold and @fgsch checks out NGINX.

@dune73 dune73 added the On Hold label Mar 4, 2019

@fgsch fgsch added On Hold and removed On Hold labels Mar 4, 2019

@fgsch

This comment has been minimized.

Copy link
Collaborator Author

fgsch commented Mar 7, 2019

Closing for now. Will reopen when I have an update.

@fgsch fgsch closed this Mar 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.