Skip to content

Commit ecc5899

Browse files
ste93cryste93cry
authored
Fix regression that send PII even when the send_default_pii option is off (#425)
1 parent ed64c02 commit ecc5899

File tree

4 files changed

+84
-5
lines changed

4 files changed

+84
-5
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
## Unreleased
44

5-
- Added missing `capture-soft-fails` config schema option (#417)
5+
- Add missing `capture-soft-fails` option to the XSD schema for the XML config (#417)
6+
- Fix regression that send PII even when the `send_default_pii` option is off (#425)
67

78
## 4.0.0 (2021-01-19)
89

‎phpstan-baseline.neon‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ parameters:
8282

8383
-
8484
message: "#^Instantiated class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseEvent not found\\.$#"
85-
count: 6
85+
count: 8
8686
path: tests/EventListener/RequestListenerTest.php
8787

8888
-

‎src/EventListener/RequestListener.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ public function handleKernelRequestEvent(RequestListenerRequestEvent $event): vo
5151
return;
5252
}
5353

54+
$client = $this->hub->getClient();
55+
56+
if (null === $client || !$client->getOptions()->shouldSendDefaultPii()) {
57+
return;
58+
}
59+
5460
$token = null;
5561
$userData = UserDataBag::createFromUserIpAddress($event->getRequest()->getClientIp());
5662

‎tests/EventListener/RequestListenerTest.php‎

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
use PHPUnit\Framework\MockObject\MockObject;
88
use PHPUnit\Framework\TestCase;
9+
use Sentry\ClientInterface;
910
use Sentry\Event;
11+
use Sentry\Options;
1012
use Sentry\SentryBundle\EventListener\RequestListener;
1113
use Sentry\State\HubInterface;
1214
use Sentry\State\Scope;
@@ -53,15 +55,19 @@ protected function setUp(): void
5355
*
5456
* @param GetResponseEvent|RequestEvent $requestEvent
5557
*/
56-
public function testHandleKernelRequestEvent($requestEvent, ?TokenInterface $token, UserDataBag $expectedUser): void
58+
public function testHandleKernelRequestEvent($requestEvent, ?ClientInterface $client, ?TokenInterface $token, ?UserDataBag $expectedUser): void
5759
{
5860
$scope = new Scope();
5961

60-
$this->tokenStorage->expects($this->once())
62+
$this->hub->expects($this->any())
63+
->method('getClient')
64+
->willReturn($client);
65+
66+
$this->tokenStorage->expects($this->any())
6167
->method('getToken')
6268
->willReturn($token);
6369

64-
$this->hub->expects($this->once())
70+
$this->hub->expects($this->any())
6571
->method('configureScope')
6672
->willReturnCallback(static function (callable $callback) use ($scope): void {
6773
$callback($scope);
@@ -83,12 +89,35 @@ public function handleKernelRequestEventForSymfonyVersionLowerThan43DataProvider
8389
return;
8490
}
8591

92+
yield 'event.requestType != MASTER_REQUEST' => [
93+
new GetResponseEvent(
94+
$this->createMock(HttpKernelInterface::class),
95+
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
96+
HttpKernelInterface::SUB_REQUEST
97+
),
98+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
99+
null,
100+
null,
101+
];
102+
103+
yield 'options.send_default_pii = FALSE' => [
104+
new GetResponseEvent(
105+
$this->createMock(HttpKernelInterface::class),
106+
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
107+
HttpKernelInterface::MASTER_REQUEST
108+
),
109+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => false])),
110+
null,
111+
null,
112+
];
113+
86114
yield 'token IS NULL' => [
87115
new GetResponseEvent(
88116
$this->createMock(HttpKernelInterface::class),
89117
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
90118
HttpKernelInterface::MASTER_REQUEST
91119
),
120+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
92121
null,
93122
UserDataBag::createFromUserIpAddress('127.0.0.1'),
94123
];
@@ -99,6 +128,7 @@ public function handleKernelRequestEventForSymfonyVersionLowerThan43DataProvider
99128
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
100129
HttpKernelInterface::MASTER_REQUEST
101130
),
131+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
102132
new class() extends AbstractToken {
103133
public function __construct()
104134
{
@@ -121,6 +151,7 @@ public function getCredentials()
121151
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
122152
HttpKernelInterface::MASTER_REQUEST
123153
),
154+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
124155
new class() extends AbstractToken {
125156
public function __construct()
126157
{
@@ -143,6 +174,7 @@ public function getCredentials()
143174
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
144175
HttpKernelInterface::MASTER_REQUEST
145176
),
177+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
146178
new class() extends AbstractToken {
147179
public function __construct()
148180
{
@@ -166,6 +198,7 @@ public function getCredentials()
166198
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
167199
HttpKernelInterface::MASTER_REQUEST
168200
),
201+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
169202
new class() extends AbstractToken {
170203
public function __construct()
171204
{
@@ -213,6 +246,7 @@ public function getCredentials()
213246
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
214247
HttpKernelInterface::MASTER_REQUEST
215248
),
249+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
216250
new class() extends AbstractToken {
217251
public function __construct()
218252
{
@@ -245,12 +279,35 @@ public function handleKernelRequestEventForSymfonyVersionAtLeast43DataProvider()
245279
return;
246280
}
247281

282+
yield 'event.requestType != MASTER_REQUEST' => [
283+
new RequestEvent(
284+
$this->createMock(HttpKernelInterface::class),
285+
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
286+
HttpKernelInterface::SUB_REQUEST
287+
),
288+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
289+
null,
290+
null,
291+
];
292+
293+
yield 'options.send_default_pii = FALSE' => [
294+
new RequestEvent(
295+
$this->createMock(HttpKernelInterface::class),
296+
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
297+
HttpKernelInterface::MASTER_REQUEST
298+
),
299+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => false])),
300+
null,
301+
null,
302+
];
303+
248304
yield 'token IS NULL' => [
249305
new RequestEvent(
250306
$this->createMock(HttpKernelInterface::class),
251307
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
252308
HttpKernelInterface::MASTER_REQUEST
253309
),
310+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
254311
null,
255312
UserDataBag::createFromUserIpAddress('127.0.0.1'),
256313
];
@@ -261,6 +318,7 @@ public function handleKernelRequestEventForSymfonyVersionAtLeast43DataProvider()
261318
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
262319
HttpKernelInterface::MASTER_REQUEST
263320
),
321+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
264322
new class() extends AbstractToken {
265323
public function __construct()
266324
{
@@ -283,6 +341,7 @@ public function getCredentials()
283341
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
284342
HttpKernelInterface::MASTER_REQUEST
285343
),
344+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
286345
new class() extends AbstractToken {
287346
public function __construct()
288347
{
@@ -305,6 +364,7 @@ public function getCredentials()
305364
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
306365
HttpKernelInterface::MASTER_REQUEST
307366
),
367+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
308368
new class() extends AbstractToken {
309369
public function __construct()
310370
{
@@ -328,6 +388,7 @@ public function getCredentials()
328388
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
329389
HttpKernelInterface::MASTER_REQUEST
330390
),
391+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
331392
new class() extends AbstractToken {
332393
public function __construct()
333394
{
@@ -375,6 +436,7 @@ public function getCredentials()
375436
new Request([], [], [], [], [], ['REMOTE_ADDR' => '127.0.0.1']),
376437
HttpKernelInterface::MASTER_REQUEST
377438
),
439+
$this->getMockedClientWithOptions(new Options(['send_default_pii' => true])),
378440
new class() extends AbstractToken {
379441
public function __construct()
380442
{
@@ -511,4 +573,14 @@ static function () {
511573
],
512574
];
513575
}
576+
577+
private function getMockedClientWithOptions(Options $options): ClientInterface
578+
{
579+
$client = $this->createMock(ClientInterface::class);
580+
$client->expects($this->any())
581+
->method('getOptions')
582+
->willReturn($options);
583+
584+
return $client;
585+
}
514586
}

0 commit comments

Comments
 (0)