Closed
Description
Summary
There are several issues with current implementation of ServletOAuth2AuthorizedClientExchangeFilterFunction.RequestContextSubscriber introduced by #6526
Actual Behavior
- RequestContextSubscriber could populate reactor Context with null values - that is forbidden since fix #1797 Explicitly reject null keys/values in Context constructors reactor/reactor-core#1800 (it is possible if Flux or Mono was subscribed outside of WebRequest or security context)
- RequestContextSubscriber replaces source context and does not respect previous values (possibly from other library)
- It does not create holder for its values in a context as it was suggested in javadoc
* Note that contexts are optimized for low cardinality key/value storage, and a user
* might want to associate a dedicated mutable structure to a single key to represent his
* own context instead of using multiple {@link #put}, which could be more costly.
* Past five user key/value pair, the {@link Context} will use a copy-on-write
* implementation backed by a new {@link java.util.Map} on each {@link #put}.
subscriber creates 4 keys (occupies almost all "optimmized" implementations of Context)
and as far as I understand CONTEXT_DEFAULTED_ATTR_NAME
key is just a marker
Maybe it would be better to put all this values into some holder so you can reduce number of keys from 4 to 1. And it allows remove extra CONTEXT_DEFAULTED_ATTR_NAME
Expected Behavior
- RequestContextSubscriber should not populate Context with null values
- RequestContextSubscriber should not replace previous Context
- RequestContextSubscriber should create DataHolder for its nullable data and do not "pollute" Context
Version
5.1.6.RELEASE
5.2.0.M4