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.

Sign up for GitHub

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

Jump to bottom

fix: copy response into reply #1

Merged
merged 1 commit into from
Dec 30, 2021
Merged

fix: copy response into reply #1

merged 1 commit into from
Dec 30, 2021

Conversation

Thom-x
Copy link
Contributor

@Thom-x Thom-x commented Dec 30, 2021

Right now, the plugin send back the whole response in the body from Modsec, not only the response body.
We are seeing other stuff like headers in the body.

HTTP/1.1 404 Not Found
Content-Length: 13238
Connection: keep-alive
Content-Type: text/html
Date: Thu, 30 Dec 2021 14:32:50 GMT
Etag: "613f2f21-33b6"
Server: nginx

<!DOCTYPE html>
<html lang="en">

With this fix we copy the headers, status and body to the response.
That way we can have the real Modsec reponse in the browser (body+headers+status).

Example for me:
image

VS

image

@Thom-x
fix: copy response into reply
5078a92 
Do not copy the whole response into the reply body but copy status headers etc.
@acouvreur acouvreur assigned acouvreur and unassigned acouvreur Dec 30, 2021
@acouvreur acouvreur self-requested a review December 30, 2021 14:58
@acouvreur
Copy link
Owner

Well now that you mention it, it seems pretty obvious.

I don't know how I didn't realize this. Thanks!

@acouvreur acouvreur merged commit 1923ea2 into acouvreur:main Dec 30, 2021
@github-actions
Copy link

🎉 This PR is included in version 1.0.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

acouvreur pushed a commit that referenced this pull request Jan 11, 2022
@Thom-x
fix: remove headers from response body (#1)
3a13902 
Do not copy the whole response into the reply body but copy status headers etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acouvreur acouvreur Awaiting requested review from acouvreur

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Thom-x @acouvreur