8000 feat(config): adds override_empty_host_header flag by M4tteoP · Pull Request #132 · coreruleset/go-ftw · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(config): adds override_empty_host_header flag #132

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

Merged
merged 9 commits into from
Mar 29, 2023

Conversation

M4tteoP
Copy link
Member
@M4tteoP M4tteoP commented Feb 21, 2023

The feature added in #63 replaces the empty host field with the destination address. It has conflicts with some CRS test that are trying to send on purpose empty or missing host headers.
It happens only when dest_addr override is in place, for this reason CRS tests against apache are still running smoothly.
This PR implements @theseion idea, adding the override_empty_host_header flag that enables this feature, permitting, by default, to keep running CRS tests as intended.
More context in the issue: #129.

Thanks the feedbacks!

tentatively closes #129

jcchavezs
jcchavezs previously approved these changes Feb 21, 2023
@jcchavezs jcchavezs requested a review from fzipi February 21, 2023 21:15
Copy link
Member
@fzipi fzipi left a comment

Choose a reason for hiding this comment

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

If we move the overrides to a new struct it will simplify things in the future...

snake66 and others added 3 commits March 28, 2023 12:16
Allows a default Host header to be set in the `testoverride` section of
the config:

```
---
testoverride:
  dest_addr: some.example.org
  input:
    headers:
      Host: another.example.org
```

It will take precedense over `dest_addr` if it is specified as well. It
will only be set if the input stage does not already have a host header
set.

Falling back to using the DestAddr override if it's not specified.
@theseion theseion requested a review from fzipi March 29, 2023 05:02
@theseion
Copy link
Collaborator

Thanks @M4tteoP, looks good. I've requested a review from @fzipi, just to be sure we're all on the same page.

Copy link
Member
@fzipi fzipi left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together! LGTM.

@jcchavezs
Copy link
Contributor
jcchavezs commented Mar 29, 2023 via email

@fzipi fzipi merged commit 35d3005 into coreruleset:main Mar 29, 2023
@camelmasa
Copy link
Contributor

Great work! If possible, Please release a new version 👍

@theseion
Copy link
Collaborator

@camelmasa
Copy link
Contributor

Thank you so much!

@M4tteoP M4tteoP deleted the adds_override_empty_host_header_flag branch May 4, 2023 13:46
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.

Conflict between missing/empty host CRS tests and setting "Host" header from dest_addr override
6 participants
0