Configuring `redirect` plugin on client level modifies the `redirect` global service

[ruudk]

We noticed an interesting behavior with the redirect plugin.

The following configuration is accepted:

$configurator->extension(
        'httplug',
        [
            'clients' => [
                'client1' => [
                    'factory' => param('httplug.factory'),
                    'plugins' => [
                        [
                            'header_defaults' => [
                                'headers' => [
                                    'User-Agent' => 'My User Agent',
                                ],
                            ],
                        ],
                        [
                            'redirect' => [
                                'preserve_header' => true,
                            ],
                        ],
                    ],
                ],
                'client2' => [
                    'factory' => param('httplug.factory'),
                    'plugins' => [
                        [
                            'header_defaults' => [
                                'headers' => [
                                    'User-Agent' => 'My User Agent',
                                ],
                            ],
                        ],
                        [
                            'redirect' => [
                                'preserve_header' => true,
                            ],
                        ],
                    ],
                ],
            ],
        ],
    );

As you can see, we configure 2 clients, and configure the redirect plugin. For the first client we set preserve_header to true and for the second to false.

We were surprised by the fact that this didn't work as expected. The reason being that the HTTPlugBundle extension overwrites the same instance httplug.plugin.redirect.

This was a bit of a foot gun for us, that took a lot of time to figure out why it wasn't working.

After reading the docs, it became clear that the redirect plugin can be configured globally. It does not say should.

We solved it by doing it like this:

$configurator->extension(
        'httplug',
        [
            'plugins' => [
                'redirect' => [
                    'preserve_header' => false,
                ],
            ],
            'clients' => [
                'client1' => [
                    'factory' => param('httplug.factory'),
                    'plugins' => [
                        [
                            'header_defaults' => [
                                'headers' => [
                                    'User-Agent' => 'My User Agent',
                                ],
                            ],
                        ],
                        'httplug.plugin.redirect',
                    ],
                ],
                'client2' => [
                    'factory' => param('httplug.factory'),
                    'plugins' => [
                        [
                            'header_defaults' => [
                                'headers' => [
                                    'User-Agent' => 'My User Agent',
                                ],
                            ],
                        ],
                        'httplug.plugin.redirect',
                    ],
                ],
            ],
        ],
    );

It's not 100% the same as before, but since it was a global plugin, we decided it would be best to have a default that is sane.

• Is there still a reason to have the redirect plugin as a global service? I think the redirect plugin was improved over time taking more configuration. For example, the plugin allows for preserve_header to be a bool or array. But the bundle does not yet accept this.
• Should we throw a big warning when someone tries to configure the redirect plugin on the client level?
• Even better: we should remove the global httplug.plugin.redirect service, and make them instances per client (like one would expect).
• Why does preserve_header default to true on the RedirectPlugin? In my opinion, this is a big mistake and also not what curl and httpie are doing when following redirects. It's could also be a security issue when sending Authorization headers to 3rd parties that you are unaware of.

I can do the work, if I get some direction on what the best solution would be.

/cc @dbu @Nyholm