8000 [HttpFoundation] Same site cookie by cvilleger · Pull Request #17 · nicolas-grekas/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] Same site cookie #17

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 11 commits into from
Closed

[HttpFoundation] Same site cookie #17

wants to merge 11 commits into from

Conversation

cvilleger
Copy link
@cvilleger cvilleger commented Apr 6, 2018 •
edited
Loading
Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets symfony#25344
License MIT
Doc PR -

TODO :

  • Symfony does not check the value of value against the characters =,; \t\r\n\013\014 whereas PHP does. Symfony only checks it against name and PHP checks it agains name and value.
  • Add test for invalid name (But should I add an Exception and test for invalid value ? BC ... )
  • When a cookie needs to be deleted, Symfony sets the max-age to 31536001 and the expiry to now
  • max-age but PHP just passes in expiry = 1 and max-age = 0.
  • PHP checks if the expiry date has a year greater than 9999.
  • Symfony also urlencode's name, I don't see PHP do that.
  • Add test for urlencode of name and value
  • Symfony uses urlencode for the name vs rawurlencode for the value, not sure why that difference is there.

nicolas-grekas
nicolas-grekas reviewed Apr 6, 2018
View reviewed changes
src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_max_age.php Outdated

$r = require __DIR__ . '/common.inc';

$r->headers->setCookie(new Cookie('foo', 'bar', 946749600, '', null, false, false));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

253402310800

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, very sorry for the time taken.

@nicolas-grekas
Copy link
Owner

@cvilleger can you push your lastest changes please?

@cvilleger
Copy link
Author

When executing ./phpunit src/Symfony/Component/HttpFoundation/Tests/ResponseFunctionalTest.php , I have one error on Max-Age attribute, trying to fix that...

1) Symfony\Component\HttpFoundation\Tests\ResponseFunctionalTest::testCookie with data set #0 ('cookie_max_age')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
     [2] => Date: Sat, 12 Nov 1955 20:04:00 GMT\n
-    [3] => Set-Cookie: foo=bar; expires=Sat, 01-Jan-10000 02:46:40 GMT; Max-Age=251878181946; path=/\n
+    [3] => Set-Cookie: foo=bar; expires=Sat, 01-Jan-10000 02:46:40 GMT; Max-Age=251878181664; path=/\n
 )\n
 shutdown\n
 '

@cvilleger cvilleger changed the title test [HttpFoundation] Same site cookie Apr 19, 2018
@nicolas-grekas
Copy link
Owner

Merged, thanks

nicolas-grekas pushed a commit that referenced this pull request Jul 27, 2022
@fabpot
feature symfony#46773 [VarDumper] Add FFI\CData and FFI\CType typ…
485843e 
…es (SerafimArts)

This PR was squashed before being merged into the 6.2 branch.

Discussion
----------

[VarDumper] Add `FFI\CData` and `FFI\CType` types

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Added support of FFI:
```php
$ffi = \FFI::cdef(<<<'CPP'
    typedef struct { int x; int y; } Point;
    typedef struct Example {
        uint8_t array[32];
        long longVal;
        __extension__ union {
            __extension__ struct { short shortVal; };
            struct { Point point; float e; };
        };
        bool boolValue;
        int (*func)(struct __sub *h);
    } Example;
CPP);

$struct = $ffi->new('Example');
$struct->func = (static fn (object $ptr) => 42);

dump($struct);
```

**Before**

```
FFI\CData {#2}
```

**After**

```
FFI\CData<struct Example> size 64 align 8 {#2
  +array: FFI\CData<uint8_t[32]> size 32 align 1 {#18}
  +int32_t longVal: 0
  +int16_t shortVal: 0
  +point: FFI\CData<struct <anonymous>> size 8 align 4 {#17
    +int32_t x: 0
    +int32_t y: 0
  }
  +float e: 0.0
  +bool boolValue: false
  +func: [cdecl] callable(struct __sub*): int32_t {#20
    returnType: FFI\CType<int32_t> size 4 align 4 {#25}
  }
}
```

P.S. I apologize for the multiple force pushes, errors in tests and codestyle have been fixed.

## Review And TODOs

- Pointers
  - [x] Pointer to scalar tests.
  - [x] Pointer to struct tests.
  - [x] "Special" pointer to `char*` tests (with `\0` and without `\0`).
- Possible Errors
  - [x] Do not dump union fields with pointer references (possible SIGSEGV).

Commits
-------

1c7dc52 [VarDumper] Add `FFI\CData` and `FFI\CType` types
nicolas-grekas pushed a commit that referenced this pull request Mar 7, 2024
@chalasr
bug symfony#54167 [Messenger] [Amqp] Handle AMQPConnectionException w…
e43b198 
…hen publishing a message. (jwage)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[Messenger] [Amqp] Handle AMQPConnectionException when publishing a message.

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix symfony#36538 Fix symfony#48241
| License       | MIT

If you have a message handler that dispatches messages to another queue, you can encounter `AMQPConnectionException` with the message "Library error: a SSL error occurred" or "a socket error occurred"  depending on if you are using tls or not or if you are running behind a load balancer or not.

You can manually reproduce this issue by dispatching a message where the handler then dispatches another message to a different queue, then go to rabbitmq admin and close the connection manually, then dispatch another message and when the message handler goes to dispatch the other message, you will get this exception:

```
a socket error occurred
#0 /vagrant/vendor/symfony/amqp-messenger/Transport/AmqpTransport.php(60): Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpSender->send()
#1 /vagrant/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php(62): Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpTransport->send()
#2 /vagrant/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php(34): Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle()
#3 /vagrant/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php(61): Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle()
#4 /vagrant/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php(41): Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle()
#5 /vagrant/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php(37): Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle()
#6 /vagrant/vendor/symfony/messenger/Middleware/TraceableMiddleware.php(40): Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle()
#7 /vagrant/vendor/symfony/messenger/MessageBus.php(70): Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle()
#8 /vagrant/vendor/symfony/messenger/TraceableMessageBus.php(38): Symfony\Component\Messenger\MessageBus->dispatch()
#9 /vagrant/src/Messenger/MessageBus.php(37): Symfony\Component\Messenger\TraceableMessageBus->dispatch()
#10 /vagrant/vendor/symfony/mailer/Mailer.php(66): App\Messenger\MessageBus->dispatch()
#11 /vagrant/src/Mailer/Mailer.php(83): Symfony\Component\Mailer\Mailer->send()
#12 /vagrant/src/Mailer/Mailer.php(96): App\Mailer\Mailer->send()
#13 /vagrant/src/MessageHandler/Trading/StrategySubscriptionMessageHandler.php(118): App\Mailer\Mailer->sendEmail()
#14 /vagrant/src/MessageHandler/Trading/StrategySubscriptionMessageHandler.php(72): App\MessageHandler\Trading\StrategySubscriptionMessageHandler->handle()
#15 /vagrant/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php(152): App\MessageHandler\Trading\StrategySubscriptionMessageHandler->__invoke()
#16 /vagrant/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php(91): Symfony\Component\Messenger\Middleware\HandleMessageMiddleware->callHandler()
#17 /vagrant/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php(71): Symfony\Component\Messenger\Middleware\HandleMessageMiddleware->handle()
#18 /vagrant/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php(34): Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle()
#19 /vagrant/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php(68): Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle()
#20 /vagrant/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php(41): Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle()
#21 /vagrant/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php(37): Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle()
#22 /vagrant/vendor/symfony/messenger/Middleware/TraceableMiddleware.php(40): Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle()
#23 /vagrant/vendor/symfony/messenger/MessageBus.php(70): Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle()
#24 /vagrant/vendor/symfony/messenger/TraceableMessageBus.php(38): Symfony\Component\Messenger\MessageBus->dispatch()
#25 /vagrant/vendor/symfony/messenger/RoutableMessageBus.php(54): Symfony\Component\Messenger\TraceableMessageBus->dispatch()
#26 /vagrant/vendor/symfony/messenger/Worker.php(162): Symfony\Component\Messenger\RoutableMessageBus->dispatch()
#27 /vagrant/vendor/symfony/messenger/Worker.php(109): Symfony\Component\Messenger\Worker->handleMessage()
#28 /vagrant/vendor/symfony/messenger/C
7CF7
ommand/ConsumeMessagesCommand.php(238): Symfony\Component\Messenger\Worker->run()
#29 /vagrant/vendor/symfony/console/Command/Command.php(326): Symfony\Component\Messenger\Command\ConsumeMessagesCommand->execute()
#30 /vagrant/vendor/symfony/console/Application.php(1096): Symfony\Component\Console\Command\Command->run()
#31 /vagrant/vendor/symfony/framework-bundle/Console/Application.php(126): Symfony\Component\Console\Application->doRunCommand()
#32 /vagrant/vendor/symfony/console/Application.php(324): Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand()
#33 /vagrant/vendor/symfony/framework-bundle/Console/Application.php(80): Symfony\Component\Console\Application->doRun()
#34 /vagrant/vendor/symfony/console/Application.php(175): Symfony\Bundle\FrameworkBundle\Console\Application->doRun()
#35 /vagrant/vendor/symfony/runtime/Runner/Symfony/ConsoleApplicationRunner.php(49): Symfony\Component\Console\Application->run()
#36 /vagrant/vendor/autoload_runtime.php(29): Symfony\Component\Runtime\Runner\Symfony\ConsoleApplicationRunner->run()
#37 /vagrant/bin/console(11): require_once('...')
#38 {main}
```

TODO:

- [x] Add test for retry logic when publishing messages

Commits
-------

f123370 [Messenger] [Amqp] Handle AMQPConnectionException when publishing a message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cvilleger @nicolas-grekas
0