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
We noticed an interesting behavior with the
redirectplugin.The following configuration is accepted:
As you can see, we configure 2 clients, and configure the
redirectplugin. For the first client we setpreserve_headertotrueand for the second tofalse.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
redirectplugin can be configured globally. It does not say should.We solved it by doing it like this:
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.
redirectplugin was improved over time taking more configuration. For example, the plugin allows forpreserve_headerto be a bool or array. But the bundle does not yet accept this.redirectplugin on the client level?httplug.plugin.redirectservice, and make them instances per client (like one would expect).preserve_headerdefault totrueon 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