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

Merge branch '2.3' of ezsystems/ezplatform-http-cache into 4.6 #55

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

ezrobot
Copy link

@ezrobot ezrobot commented Nov 28, 2024

Cross merge PR

@alongosz
Copy link
Member

@glye you have conflicts here.

Does github now generate those commits like "Merge commit from fork"? Keep in mind that commit like this won't be visible in auto-generated changelogs. However we can merge this PR adding IBX-XXX: prefix.

@glye
Copy link
Contributor

glye commented Nov 28, 2024

@alongosz The "Merge commit from fork" is automatic, yes.

@glye
Copy link
Contributor

glye commented Nov 28, 2024

@alongosz Manual merge done, hope I didn't 🪛 it up.

@alongosz
Copy link
Member

@alongosz The form the merge should take is https://github.com/ibexa/http-cache-ghsa-fh7v-q458-7vmw/pull/1/files

I don't have access to it @glye (:

@glye
Copy link
Contributor

glye commented Nov 28, 2024

@alongosz Sorry, I removed it already. But I'm pretty sure I got it right :)

Copy link
Contributor

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

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

I wasn't able to dig deep enough to have some further context on compression, but it feels like we are skipping it for some important content types (REST-related, JSON). I wonder if that has any repercussions on performance but I guess that wasn't checked, wasn't that? +1 to unblock release.

@glye
Copy link
Contributor

glye commented Nov 28, 2024

Yes, there can be a performance impact, but fixing it in other ways than turning off compression is a hard problem to solve, with security pitfalls. Ref. https://www.breachattack.com/
We can discuss it after release, agree to unblock release.

@konradoboza
Copy link
Contributor

konradoboza commented Nov 28, 2024

At least rate-limiting looks like potential low-hanging fruit, but this still only reduces the breach probability. Thanks for the details @glye.

@glye glye merged commit e03f683 into 4.6 Nov 28, 2024
24 checks passed
@glye glye deleted the temp_2.3_to_4.6 branch November 28, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants