laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.23k stars 10.92k forks source link

Content-Type header duplicated when calling multiple similar methods on the Http facade. #36430

Closed daronspence closed 3 years ago

daronspence commented 3 years ago

Description:

When calling multiple methods on the HTTP facade that manipulate similar headers for the request, the Content-Type header is not overwritten with the most recent value.

Steps To Reproduce:

If you inspect the object of this chain, you will see what I'm talking about.

Http::asForm()->asJson()->asForm()->asJson()
/*
Illuminate\Http\Client\PendingRequest {#1485 ▼
  #factory: Illuminate\Http\Client\Factory {#1481 ▶}
  #baseUrl: ""
  #bodyFormat: "json"
  #pendingBody: null
  #pendingFiles: []
  #cookies: null
  #transferStats: null
  #options: array:2 [▼
    "http_errors" => false
    "headers" => array:1 [▼
      "Content-Type" => array:4 [▼
        0 => "application/x-www-form-urlencoded"
        1 => "application/json"
        2 => "application/x-www-form-urlencoded"
        3 => "application/json"
      ]
    ]
  ]
  #tries: 1
  #retryDelay: 100
  #beforeSendingCallbacks: Illuminate\Support\Collection {#1475 ▶}
  #stubCallbacks: Illuminate\Support\Collection {#2222 ▶}
  #middleware: Illuminate\Support\Collection {#2223 ▶}
}
*/

In this scenario, I would expect the Content-Type to be equal to the last set value for that header; application/json in this case.

I know this example is a little contrived, but in my app, I had an Http client configuration that I wanted to override to a different Content-Type for one request. I expected to be able to call the appropriate method to change the content type but it didn't work.

As a workaround I created a macro to clear the content type header.

PendingRequest::macro('clearContentType', function () {
  $this->options['headers']['Content-Type'] = [];
  return $this;
});

Is this expected behavior?

Thanks!

driesvints commented 3 years ago

While I do admit that's a little unfortunate it's not meant that you chain the asX methods like that. You'd want to indicate a Http request as one or the other, not toggle between those states. In that case it's probably best that you initiate a new Http call instance all together.

However, if you could somehow send in a PR that fixes this in a clean way than we could consider that. Thanks