tower-rs / tower-http

HTTP specific Tower utilities.
675 stars 156 forks source link

enable cors/tests.rs and make it pass #473

Closed GlenDC closed 6 months ago

GlenDC commented 6 months ago

Motivation

cors/tests.rs was added somewhere between now and the 0.5 release. However the file wasn't enabled, and this seemed to also hide a bug, where we did not in fact preserve the already set Vary (http response) header.

Found while porting over the updated/added logic since 0.5 to https://github.com/plabayo/rama

(as that codebase uses ported code from tower-async, which is ported from tower, since then I have abandoned tower-async as it did its job to proof that it was possible, but more drastic changes were desired on my side for rama. I do keep however rama in sync with improvements made to codebases such as tower-http, which is one of many reasons why I love to contribute to tower where I can and where it is desired)

Solution

  1. enable the test by adding the mod to cors/mod.rs
  2. fix the bug in cors/mod.rs where we remove from headers instead of response_headers

As flagged on Discord it does also mean we need a third change where the order is not preserved, and instead the custom (already set) Vary header will be at the end.