Issue #43: Control the contents of extra in error log.
#44
Conversation
Signed-off-by: alexmerlin <[email protected]>
NoteI will write tests after we agree on the final form of the features (and their behaviour) implemented in this PR. |
Signed-off-by: alexmerlin <[email protected]>
composer.json
Outdated
| @@ -29,6 +29,7 @@ | |||
| "php": "~8.2.0 || ~8.3.0 || ~8.4.0", | |||
| "dotkernel/dot-log": "^4.1.0", | |||
| } elseif (is_string($value)) { | ||
| $lowerKey = strtolower($key); | ||
| if ( | ||
| str_contains($lowerKey, 'password') || |
There was a problem hiding this comment.
how if we move this array-key into config ?
in a forbidden-string array
to allow the developer to define from the beginning what key to not save in log files
There was a problem hiding this comment.
@arhimede After I tried to implement this, I still think that we need a better solution for this type of filtering.
For exact key search, developers must add to the forbidden-string array each sensitive data related field name (password, passwordConfirm, userPasswordCsrf, contactFormCsrf) manually - which is error prone, as they might create a new form and forget to add the new key(s) to the array.
For partial key search (anything that matches password, key, csrf, token etc.), large forms with many fields might become an overkill to detect all keys.
Take for example a large...r form, with 50 fields.
We need to check each form field name against the forbidden-string array (that can have maybe 20 items), that's 50x20=1000 checks.
With larger forms, to me it seems overkill just for logging an error.
There was a problem hiding this comment.
I agree is overkill ...
Can we disable by default this logger key then and warn the developer that if he enable it , can be a lot of issues
There was a problem hiding this comment.
I managed to optimize it so I kept this feature.
- Documentation - Tests Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
|
🤯 Fun fact: Running Psalm on the latest version of Psalm: Running Psalm on version (6.6.2) before the faulty version of Psalm: |
|
Created issue at Psalm: |
Signed-off-by: alexmerlin <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.1 #44 +/- ##
======================================
Coverage ? 98.46%
Complexity ? 117
======================================
Files ? 20
Lines ? 326
Branches ? 0
======================================
Hits ? 321
Misses ? 5
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
Signed-off-by: alexmerlin <[email protected]>
|
@MarioRadu @pinclau @Jurj-Bogdan If you have time for a review... |
|
modify readme.md fiwl to follow this model Package Name H1 description wich use the PSr-15 keyword Documentation H2 Documentation is available at: Badges H2 on top of badges lines and do this in documentation first page too |
Signed-off-by: alexmerlin <[email protected]>
|
@arhimede Added the requested items in the last commit. |
No description provided.