Log exceptions as nested array instead of encoded json by icewind1991 · Pull Request #8946 · nextcloud/server

icewind1991 · 2018-03-22T14:55:18Z

Revived version of #7826

Logreader support: nextcloud/logreader@0f4be88

I would also like to extend the amount of data we log in a follow up pr (mainly "preview" exception)

MorrisJobke · 2018-03-22T15:08:11Z

There was one problem ... I only stated that it is because of the type hinting. I guess what we hand in into the log implementations. But this looks good so far in your version. 🤔

icewind1991 · 2018-04-05T14:15:58Z

From what I understood the problem was the old logException calling log which is now type-hinted to make calling it with the nested array invalid. Thus I refactored the logging so logException does not need to call log

rullzer

Fine by me

MorrisJobke · 2018-04-09T21:35:20Z

lib/private/Log.php

 			'Message' => $exception->getMessage(),
 			'Code' => $exception->getCode(),
-			'Trace' => $exception->getTraceAsString(),
+			'Trace' => $exception->getTrace(),


If this is used passwords are still logged 🙈

{ "reqId": "xaT0UndUeGLdPMu2jnI6", "level": 3, "time": "2018-04-09T21:18:30+00:00", "remoteAddr": "172.20.0.1", "user": "--", "app": "index", "method": "POST", "url": "/index.php/login", "message": { "CustomMessage": "--", "Exception": "InvalidArgumentException", "Message": "Username must be string", "Code": 0, "Trace": [ { "file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php", "line": 166, "function": "tryLogin", "class": "OC\\Core\\Controller\\LoginController", "type": "->", "args": [ "admin", "password", null, false, "Europe/Berlin", "2" ] }, { "file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php", "line": 99, "function": "executeController", "class": "OC\\AppFramework\\Http\\Dispatcher", "type": "->", "args": [ {}, "tryLogin" ] }, { "file": "/var/www/html/lib/private/AppFramework/App.php", "line": 118, "function": "dispatch", "class": "OC\\AppFramework\\Http\\Dispatcher", "type": "->", "args": [ {}, "tryLogin" ] }, { "file": "/var/www/html/lib/private/AppFramework/Routing/RouteActionHandler.php", "line": 47, "function": "main", "class": "OC\\AppFramework\\App", "type": "::", "args": [ "OC\\Core\\Controller\\LoginController", "tryLogin", {}, { "_route": "core.login.tryLogin" } ] }, { "function": "__invoke", "class": "OC\\AppFramework\\Routing\\RouteActionHandler", "type": "->", "args": [ { "_route": "core.login.tryLogin" } ] }, { "file": "/var/www/html/lib/private/Route/Router.php", "line": 297, "function": "call_user_func", "args": [ {}, { "_route": "core.login.tryLogin" } ] }, { "file": "/var/www/html/lib/base.php", "line": 981, "function": "match", "class": "OC\\Route\\Router", "type": "->", "args": [ "/login" ] }, { "file": "/var/www/html/index.php", "line": 37, "function": "handleRequest", "class": "OC", "type": "::", "args": [] } ], "File": "/var/www/html/core/Controller/LoginController.php", "Line": 245 }, "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/604.5.6 (KHTML, like Gecko) Version/11.0.3 Safari/604.5.6", "version": "14.0.0.1" }

codecov · 2018-04-10T09:02:42Z

Codecov Report

Merging #8946 into master will increase coverage by 0.02%.
The diff coverage is 91.07%.

@@             Coverage Diff              @@
##             master    #8946      +/-   ##
============================================
+ Coverage     51.92%   51.94%   +0.02%     
- Complexity    25313    25344      +31     
============================================
  Files          1604     1603       -1     
  Lines         95135    95218      +83     
  Branches       1388     1387       -1     
============================================
+ Hits          49402    49465      +63     
- Misses        45733    45753      +20

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Log/File.php	`56.04% <ø> (ø)`	`30 <0> (ø)`	⬇️
lib/private/Log.php	`82.75% <91.07%> (+2.53%)`	`52 <26> (+13)`	⬆️
lib/private/Security/Hasher.php	`85.71% <0%> (-11.51%)`	`24% <0%> (+6%)`
apps/dav/lib/HookManager.php	`42.02% <0%> (-9.76%)`	`20% <0%> (+5%)`
apps/admin_audit/lib/Actions/UserManagement.php	`6.25% <0%> (-2.85%)`	`9% <0%> (+2%)`
lib/private/Files/Cache/Propagator.php	`94.93% <0%> (-1.27%)`	`16% <0%> (ø)`
apps/user_ldap/lib/Jobs/Sync.php	`64.77% <0%> (-0.38%)`	`46% <0%> (ø)`
apps/files_trashbin/lib/Trashbin.php	`72.46% <0%> (-0.25%)`	`136% <0%> (ø)`
apps/admin_audit/lib/AppInfo/Application.php	`8.39% <0%> (-0.14%)`	`20% <0%> (ø)`
apps/user_ldap/lib/Access.php	`36.36% <0%> (-0.03%)`	`326% <0%> (+2%)`
... and 10 more

icewind1991 · 2018-04-10T09:04:11Z

@MorrisJobke fixed

MorrisJobke · 2018-04-10T09:13:16Z

Now it removes all parameters:

{
    "reqId": "WW1k9pgFAakfRJvaGxGD",
    "level": 3,
    "time": "2018-04-10T09:12:17+00:00",
    "remoteAddr": "172.20.0.1",
    "user": "--",
    "app": "index",
    "method": "POST",
    "url": "/index.php/login",
    "message": {
        "CustomMessage": "--",
        "Exception": "InvalidArgumentException",
        "Message": "Username must be string",
        "Code": 0,
        "Trace": [
            {
                "file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php",
                "line": 166,
                "function": "tryLogin",
                "class": "OC\\Core\\Controller\\LoginController",
                "type": "->",
                "args": [
                    "*** sensitive parameter replaced ***"
                ]
            },
            {
                "file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php",
                "line": 99,
                "function": "executeController",
                "class": "OC\\AppFramework\\Http\\Dispatcher",
                "type": "->",
                "args": [
                    "*** sensitive parameter replaced ***",
                    "*** sensitive parameter replaced ***"
                ]
            },
            {
                "file": "/var/www/html/lib/private/AppFramework/App.php",
                "line": 118,
                "function": "dispatch",
                "class": "OC\\AppFramework\\Http\\Dispatcher",
                "type": "->",
                "args": [
                    "*** sensitive parameter replaced ***",
                    "*** sensitive parameter replaced ***"
                ]
            },
            {
                "file": "/var/www/html/lib/private/AppFramework/Routing/RouteActionHandler.php",
                "line": 47,
                "function": "main",
                "class": "OC\\AppFramework\\App",
                "type": "::",
                "args": [
                    "*** sensitive parameter replaced ***",
                    "*** sensitive parameter replaced ***",
                    "*** sensitive parameter replaced ***",
                    "*** sensitive parameter replaced ***"
                ]
            },
            {
                "function": "__invoke",
                "class": "OC\\AppFramework\\Routing\\RouteActionHandler",
                "type": "->",
                "args": [
                    "*** sensitive parameter replaced ***"
                ]
            },
            {
                "file": "/var/www/html/lib/private/Route/Router.php",
                "line": 297,
                "function": "call_user_func",
                "args": [
                    "*** sensitive parameter replaced ***",
                    "*** sensitive parameter replaced ***"
                ]
            },
            {
                "file": "/var/www/html/lib/base.php",
                "line": 982,
                "function": "match",
                "class": "OC\\Route\\Router",
                "type": "->",
                "args": [
                    "*** sensitive parameter replaced ***"
                ]
            },
            {
                "file": "/var/www/html/index.php",
                "line": 37,
                "function": "handleRequest",
                "class": "OC",
                "type": "::",
                "args": []
            }
        ],
        "File": "/var/www/html/core/Controller/LoginController.php",
        "Line": 243
    },
    "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.1 Safari/605.1.15",
    "version": "14.0.0.2"
}

icewind1991 · 2018-04-10T09:29:38Z

missed a strict option 🙈

Signed-off-by: Robin Appelman <robin@icewind.nl>

MorrisJobke

Tested and works 👍

MorrisJobke · 2018-04-10T21:54:44Z

Unit tests fail 🙈

Signed-off-by: Robin Appelman <robin@icewind.nl>

icewind1991 · 2018-04-11T10:57:25Z

ci is happy

icewind1991 added the 3. to review Waiting for reviews label Mar 22, 2018

icewind1991 added this to the Nextcloud 14 milestone Mar 22, 2018

icewind1991 requested review from MorrisJobke, nickvergessen and rullzer March 22, 2018 14:55

rullzer approved these changes Apr 9, 2018

View reviewed changes

MorrisJobke reviewed Apr 9, 2018

View reviewed changes

icewind1991 force-pushed the log-exceptions-as-nested-array-2 branch from 632c2f2 to 5be0255 Compare April 10, 2018 09:02

icewind1991 force-pushed the log-exceptions-as-nested-array-2 branch from 5be0255 to 58526d6 Compare April 10, 2018 09:29

Log exceptions as nested array instead of encoded json

b9583c6

Signed-off-by: Robin Appelman <robin@icewind.nl>

icewind1991 force-pushed the log-exceptions-as-nested-array-2 branch from 58526d6 to b9583c6 Compare April 10, 2018 13:58

MorrisJobke approved these changes Apr 10, 2018

View reviewed changes

MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 10, 2018

adjust tests to new exception log format

15815c0

Signed-off-by: Robin Appelman <robin@icewind.nl>

icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 11, 2018

MorrisJobke approved these changes Apr 11, 2018

View reviewed changes

MorrisJobke merged commit 7a3ce07 into master Apr 11, 2018

MorrisJobke deleted the log-exceptions-as-nested-array-2 branch April 11, 2018 12:24

MorrisJobke mentioned this pull request Apr 11, 2018

Critical changes for developers & admins for Nextcloud 14 #7827

Closed

ChristophWurst mentioned this pull request Dec 5, 2022

Fix array to string conversion in errorlog writer #35614

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Log exceptions as nested array instead of encoded json#8946

Log exceptions as nested array instead of encoded json#8946
MorrisJobke merged 2 commits intomasterfrom
log-exceptions-as-nested-array-2

icewind1991 commented Mar 22, 2018

Uh oh!

MorrisJobke commented Mar 22, 2018

Uh oh!

icewind1991 commented Apr 5, 2018

Uh oh!

rullzer left a comment

Uh oh!

MorrisJobke Apr 9, 2018

Uh oh!

codecov bot commented Apr 10, 2018 •

edited

Loading

Uh oh!

icewind1991 commented Apr 10, 2018

Uh oh!

MorrisJobke commented Apr 10, 2018

Uh oh!

icewind1991 commented Apr 10, 2018

Uh oh!

MorrisJobke left a comment

Uh oh!

MorrisJobke commented Apr 10, 2018

Uh oh!

icewind1991 commented Apr 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

icewind1991 commented Mar 22, 2018

Uh oh!

MorrisJobke commented Mar 22, 2018

Uh oh!

icewind1991 commented Apr 5, 2018

Uh oh!

rullzer left a comment

Choose a reason for hiding this comment

Uh oh!

MorrisJobke Apr 9, 2018

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

icewind1991 commented Apr 10, 2018

Uh oh!

MorrisJobke commented Apr 10, 2018

Uh oh!

icewind1991 commented Apr 10, 2018

Uh oh!

MorrisJobke left a comment

Choose a reason for hiding this comment

Uh oh!

MorrisJobke commented Apr 10, 2018

Uh oh!

icewind1991 commented Apr 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Apr 10, 2018 •

edited

Loading