[WFLY-15452] - modify ajp-listener to allow specifying pattern for aj… #435
Conversation
| @@ -0,0 +1,121 @@ | |||
| = <INSERT TITLE HERE> | |||
There was a problem hiding this comment.
It appears you have the title at the wrong line :-)
|
|
||
| == Overview | ||
|
|
||
| Since UNDERTOW-1667 one can set additional AJP request attribute parsing permission via env variable. However there is no way to set it in WFLY config nor directly in UndertowOptions. Later is being addressed in UNDERTOW-1978, to which this RFE is a follow up. |
There was a problem hiding this comment.
We need to update this, since the UndertowOptions is also already available without any PRs for UNDERTOW-1978
|
|
||
| * [x] Managed domain | ||
|
|
||
| * [~x] OpenShift s2i |
|
|
||
| === Hard Requirements | ||
|
|
||
| * Being able to configure pattern via XML configuration file. |
There was a problem hiding this comment.
via model, it could be directly in the XML file or it could be via CLI
|
|
||
| Possibly. Subsystem transformers should be able to handle it. | ||
|
|
||
| === Default Configuration |
There was a problem hiding this comment.
I think default configuration refers to what will be the default configuration of the server, can someone vouch if I'm right?
So, by default, are we having a allowed_request_attr_pattern in the server? Nope, because we don't have an ajp-listener. Still, the example of the xml could be valid in Hard Requirements, and very helpful for the test plan
There was a problem hiding this comment.
I prefer "standard configuration" when describing the standalone/host/domain.xml files we include in our standard distribution zip.
"Default" is ambiguous because many attributes have default values but those are only relevant is a resource with is configured. Plus, our standard configurations may have a value for an attribute that is different from its default value. So I prefer to only use 'default' to refer to default attribute behavior.
There was a problem hiding this comment.
Perhaps the default value (of the new attribute) could be undefined as in when defining the attribute definition, you don't use the SimpleAttributeDefinitionBuilder.setDefaultValue() function. And the meaning of that is that there is no allowed_request_attr_pattern, even is ajp-listener is defined. Or if there is a default value recommended by the specification (or a value that is commonly used by other such solutions), then we can use that as the default value.
And as for the standard configuration, if the ajp-listener is not defined by default, the value for allowed_request_attr_pattern would also be undefined regardless of what we used for SimpleAttributeDefinitionBuilder.setDefaultValue()
WDYT? @fl4via @baranowb @bstansberry
There was a problem hiding this comment.
@PrarthonaPaul looking at https://github.com/wildfly/wildfly/pull/14789/files I see the attribute has no default value, so it looks like what you describe is what was implemented.
AIUI the effect of that is if the ajp-listener is configured with this feature in place the listener behavior is no different from before; i.e. undertow will reject unknown request attributes. So this feature has no 'Default configuration' compatibility concerns.
There was a problem hiding this comment.
sounds good! Thanks @bstansberry
There was a problem hiding this comment.
There was a problem hiding this comment.
Hi @baranowb
The proposal looks good! I have added some minor comments, please feel free to let me know if you have any questions.
|
|
||
| Possibly. Subsystem transformers should be able to handle it. | ||
|
|
||
| === Default Configuration |
There was a problem hiding this comment.
Perhaps the default value (of the new attribute) could be undefined as in when defining the attribute definition, you don't use the SimpleAttributeDefinitionBuilder.setDefaultValue() function. And the meaning of that is that there is no allowed_request_attr_pattern, even is ajp-listener is defined. Or if there is a default value recommended by the specification (or a value that is commonly used by other such solutions), then we can use that as the default value.
And as for the standard configuration, if the ajp-listener is not defined by default, the value for allowed_request_attr_pattern would also be undefined regardless of what we used for SimpleAttributeDefinitionBuilder.setDefaultValue()
WDYT? @fl4via @baranowb @bstansberry
…p request attributes
|
Looks good to me! |
|
Thanks @baranowb @fl4via and @PrarthonaPaul |
| // Choose the planned stability level for the proposed functionality | ||
| * [ ] Experimental | ||
|
|
||
| * [X] Preview |
There was a problem hiding this comment.
Is this correct? IMO we are aiming for dafault with EAP7-1836.
|
|
||
| == Backwards Compatibility | ||
|
|
||
| Possibly. Subsystem transformers should be able to handle it. |
There was a problem hiding this comment.
@baranowb @bstansberry may we (Migration feature team) assume that if there is any need for migration this will be handled by the parsers/transformers. "Possibly" is not really reassuring that this feature team is fully handling migration on its own.
There was a problem hiding this comment.
| Parameters will be present in undertow server element(for standalone: /subsystem=undertow/server=default-server/ajp-listener=myListener): | ||
| * allowed_request_attr_pattern | ||
| ** Default: null | ||
| ** Type: String(regex - java.util.regex.Pattern) |
There was a problem hiding this comment.
What should be an outcome when custom attribute doesn't match the pattern? Should it be ignored, but still processed (200) or should the listener return 403?
…p request attributes
Issue: https://issues.redhat.com/browse/WFLY-15452