GH-3938: Fix HTTP XML configuration for ambiguity #3939
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes spring-projects#3938 The `encoding-mode` is a property of the `RestTemplate`. Therefore, it cannot be set on the component configuration together with an externally injected `rest-template` Even if `HttpRequestExecutingMessageHandler` has an assertion for such an ambiguity, the XML parser just ignores this `encoding-mode` when it encounters the `rest-template` * Fix `HttpAdapterParsingUtils.verifyNoRestTemplateAttributes()` to check for not allowed attributes with a `encoding-mode` as well * Remove a `default` from the `encoding-mode` to not cause an ambiguity in the parser * Fix some typos in the `spring-integration-http.xsd` * Rework `OutboundResponseTypeTests` to JUnit 5 **Cherry-pick to `5.5.x`**
garyrussell
suggested changes
Nov 7, 2022
|
|
||
| static final String[] SYNC_REST_TEMPLATE_REFERENCE_ATTRIBUTES = { | ||
| "request-factory", "error-handler", "message-converters" | ||
| "request-factory", "error-handler", "message-converters", "encoding-mode" |
There was a problem hiding this comment.
Looks like this will break the for loops that use this constant (gateway and adapter parsers). (Set reference instead of value). We don't appear to have test coverage for this condition.
I also think we need to improve the error message to say that this property must be set on the template's UriBuilderFactory, whereas the other 3 are set on the template itself.
…ils` * Cover `encoding-mode` and `rest-template` ambiguity with a test against failing XML configuration
|
Could we also improve parserContext.getReaderContext().error("When providing a 'rest-template' reference, the '"
+ attributeName + "' attribute is not allowed.",
parserContext.extractSource(element));to parserContext.getReaderContext().error("When providing a 'rest-template' reference, the '"
+ attributeName + "' attribute is not allowed, it must be set on the provided template instead",
parserContext.extractSource(element));?? |
garyrussell
pushed a commit
that referenced
this pull request
Nov 7, 2022
* GH-3938: Fix HTTP XML configuration for ambiguity Fixes #3938 The `encoding-mode` is a property of the `RestTemplate`. Therefore, it cannot be set on the component configuration together with an externally injected `rest-template` Even if `HttpRequestExecutingMessageHandler` has an assertion for such an ambiguity, the XML parser just ignores this `encoding-mode` when it encounters the `rest-template` * Fix `HttpAdapterParsingUtils.verifyNoRestTemplateAttributes()` to check for not allowed attributes with a `encoding-mode` as well * Remove a `default` from the `encoding-mode` to not cause an ambiguity in the parser * Fix some typos in the `spring-integration-http.xsd` * Rework `OutboundResponseTypeTests` to JUnit 5 **Cherry-pick to `5.5.x`** * * Fix error handling for `encoding-mode` in the `HttpAdapterParsingUtils` * Cover `encoding-mode` and `rest-template` ambiguity with a test against failing XML configuration * * Improve error message for ambiguous attributes in the `HttpAdapterParsingUtils`
|
... and cherry-picked as e4e8041 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3938
The
encoding-modeis a property of theRestTemplate. Therefore, it cannot be set on the component configuration together with an externally injectedrest-templateEven if
HttpRequestExecutingMessageHandlerhas an assertion for such an ambiguity, the XML parser just ignores thisencoding-modewhen it encounters therest-templateHttpAdapterParsingUtils.verifyNoRestTemplateAttributes()to check for not allowed attributes with aencoding-modeas welldefaultfrom theencoding-modeto not cause an ambiguity in the parserspring-integration-http.xsdOutboundResponseTypeTeststo JUnit 5Cherry-pick to
5.5.x