Respect mod_unique_id and refactor OC_Request::getRequestId
#13973
Conversation
When `mod_unique_id` is enabled the ID generated by it will be used for logging. This allows for correlation of the Apache logs and the ownCloud logs. Testplan: - [ ] When `mod_unique_id` is enabled the request ID equals the one generated by `mod_unique_id`. - [ ] When `mod_unique_id` is not available the request ID is a 20 character long random string - [ ] The generated Id is stable over the lifespan of one request Changeset looks a little bit larger since I had to adjust every unit test using the HTTP\Request class for proper DI. Fixes #13366
LukasReschke
force-pushed
the
enhancement/security/13366
branch
from
9e5ca32
to
770fa76
Compare
|
@owncloud-bot Retest this please |
|
Look good thx 👍 |
|
Refer to this link for build results (access rights to CI server needed): |
|
Feel free to submit more prs to kill oc_request completely. |
With pleasure. – Let's get this in first though to prevent merge fuckups. I didn't want to mix new features with refactoring in one PR. @nickvergessen Care to review as well? |
|
Refer to this link for build results (access rights to CI server needed): |
|
The inspection completed: 6 new issues, 3 updated code elements |
| 'method' => 'hi', | ||
| ) | ||
| ], | ||
| $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock() |
There was a problem hiding this comment.
This is equal to $this->getMock('\OCP\Security\ISecureRandom') just as a hint.
|
Looks good 👍 |
Respect `mod_unique_id` and refactor `OC_Request::getRequestId`
This changeset removes the static class `OC_Request` and moves the functions either into `IRequest` which is accessible via `\OC::$server::->getRequest()` or into a separated `TrustedDomainHelper` class for some helper methods which should not be publicly exposed. This changes only internal methods and nothing on the public API. Some public functions in `util.php` have been deprecated though in favour of the new non-static functions. Unfortunately some part of this code uses things like `__DIR__` and thus is not completely unit-testable. Where tests where possible they ahve been added though. Fixes #13976 which was requested in #13973 (comment)
This changeset removes the static class `OC_Request` and moves the functions either into `IRequest` which is accessible via `\OC::$server::->getRequest()` or into a separated `TrustedDomainHelper` class for some helper methods which should not be publicly exposed. This changes only internal methods and nothing on the public API. Some public functions in `util.php` have been deprecated though in favour of the new non-static functions. Unfortunately some part of this code uses things like `__DIR__` and thus is not completely unit-testable. Where tests where possible they ahve been added though. Fixes #13976 which was requested in #13973 (comment)
This changeset removes the static class `OC_Request` and moves the functions either into `IRequest` which is accessible via `\OC::$server::->getRequest()` or into a separated `TrustedDomainHelper` class for some helper methods which should not be publicly exposed. This changes only internal methods and nothing on the public API. Some public functions in `util.php` have been deprecated though in favour of the new non-static functions. Unfortunately some part of this code uses things like `__DIR__` and thus is not completely unit-testable. Where tests where possible they ahve been added though. Fixes #13976 which was requested in #13973 (comment)
|
From the Git history, this commit introduced |
|
@drzraf I don't see any reference of "OC_MTIME" in this diff, maybe you commented on the wrong PR ? I suggest you make a new ticket to discuss this. |
|
commt 886bda5
Still I don't see documentation about timestamp preservation and how one is expected to |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
When
mod_unique_idis enabled the ID generated by it will be used for logging. This allows for correlation of the Apache logs and the ownCloud logs.Testplan:
mod_unique_idis enabled the request ID equals the one generated bymod_unique_id.mod_unique_idis not available the request ID is a 20 character long random stringChangeset looks a little bit larger since I had to adjust every unit test using the HTTP\Request class for proper DI.
Fixes #13366
@MorrisJobke @PVince81 Please review.