Update OptionsFilter to not emit duplicate headers

[jeremyg484]

OptionsFilter is updated to act upon the response and ensure that the Allow header is only written one time.

The filter was previously not working as expected with any of the Servlet implementations (including cloud
functions). The problem being that an initial response has already been generated in the onRouteMiss method
of RequestLifecycle and the Allow header has already been set using the same route lookup logic that was
implemented in this filter. The result was that there would be multiple Allow headers emitted, containing mostly
duplicate values.

The approach I've taken is to turn this filter into a ResponseFilter that relies on the fact that the allowed
methods have already been set in the RequestLifecycle if there is not any explicit Options route implemented by
the user.

Ultimately this simplifies things a bit, as we no longer need to look up the allowed routes here in the filter.

Additionally, this ensures that the Allow header will only be emitted once, with a comma separated list of
acceptable methods. The implementation was previously writing out multiple Allow headers each with a single method
value. While this is technically allowed by the HTTP spec, I think single multi-valued header format is
cleaner and less surprising.

This should clear the way further for being able to successfully update the Micronaut Core dependency to 4.2.x in Servlet, GCP, AWS, etc.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

90.0% Coverage
0.0% Duplication

[sdelamo]

don't we have a getAll method to simplify this

[jeremyg484]

IMO, getAll makes it more complicated because it does not do any parsing of the values, it just returns the raw value strings. So in this case, the header has already been written in RequestLifecycle as something like Allow: GET,POST,HEAD. getAll then returns a list with a single String value of "GET,POST,HEAD". Thus to produce this same result we would need conditional logic to check whether that String is empty to determine whether to add a comma before appending OPTIONS to it.

[wetted]

I had to disable the OptionsFilterTest on for the servlet TCK on micronaut-projects/micronaut-servlet#588. That should be re-enabled once this is merged.

[yawkat]

at this point might as well move the entire logic into RequestLifecycle or wherever METHOD_NOT_ALLOWED is thrown...

[jeremyg484]

@yawkat Yes, I had that same thought. It happens in RequestLifecycle#onRouteMiss.

My main concern, and reason for not doing that yet, is that onRouteMiss executes before runWithFilters and the original intention of this filter to avoid conflicting with the CORS filter.

That said, it occurs to me now that the onRouteMiss logic must be getting executed anyway (and the Allow header getting populated) even in the case of CORS being enabled. Thus in a Servlet environment, we are likely sending the Allow header even in response to a CORS pre-flight request (alongside all the other headers that CORS adds) because of the same issue of not being able to create a new ServletResponse in the middle of request processing.