-
Notifications
You must be signed in to change notification settings - Fork 166
Improve xml parsing in appsec #3558
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
Conversation
da304ff to
0650038
Compare
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06500381d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (71.08%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #3558 +/- ##
==========================================
+ Coverage 61.79% 61.90% +0.10%
==========================================
Files 139 140 +1
Lines 13114 13281 +167
Branches 1728 1758 +30
==========================================
+ Hits 8104 8221 +117
- Misses 4240 4272 +32
- Partials 770 788 +18
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
de228f3 to
df15b71
Compare
* Partial XML supporterd. * No more PHP code run during RINIT, which is problematic for some 3rd party extensions. Instead libxml2 is bundled and called directly.
df15b71 to
71e9728
Compare
630095c to
ef4a88e
Compare
|
Hey @cataphract, just for my understanding:
I might just not be aware of it (but I assume as there are other third party deps already) if there is some kind of process that helps us in monitor those upstream dependencies for new releases / bug fixes / security issues? |
I don't think so. We don't compile together with PHP, so we don't know which version of the headers of libxml PHP used. Therefore we could have mismatch between the data structures in that runtime and build time versions.
This is add a bit of delay on downloading the submodule and prevents us from changing patching configuration files... However, I'll improve the vendoring script so it doesn't require extensive modifications (e.g. in the file list) for each new version. There's also the disadvantage the build would break not just when github is down but also when the gitlab instacne of gnome is down.
I'm not sure about any such process... But we could add a check during CI that we're on the latest version of libxml2 |
Description
Reviewer checklist