[
https://issues.apache.org/jira/browse/KNOX-3007?focusedWorklogId=905946&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-905946
]
ASF GitHub Bot logged work on KNOX-3007:
----------------------------------------
Author: ASF GitHub Bot
Created on: 20/Feb/24 13:53
Start Date: 20/Feb/24 13:53
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #841:
URL: https://github.com/apache/knox/pull/841#discussion_r1495847968
##########
gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactoryTest.java:
##########
@@ -209,6 +210,7 @@ public void testHttpClientPathNormalization() {
GatewayConfig gatewayConfig = createMock(GatewayConfig.class);
expect(gatewayConfig.getHttpClientConnectionTimeout()).andReturn(20000).once();
expect(gatewayConfig.getHttpClientSocketTimeout()).andReturn(20000).once();
+
expect(gatewayConfig.getHttpClientCookieSpec()).andReturn("standard").anyTimes();
Review Comment:
I'd use org.apache.http.client.config.CookieSpecs.STANDARD here.
##########
gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactoryTest.java:
##########
@@ -61,6 +61,7 @@ public void testCreateHttpClientSSLContextDefaults() throws
Exception {
expect(gatewayConfig.getHttpClientMaxConnections()).andReturn(32).once();
expect(gatewayConfig.getHttpClientConnectionTimeout()).andReturn(20000).once();
expect(gatewayConfig.getHttpClientSocketTimeout()).andReturn(20000).once();
+
expect(gatewayConfig.getHttpClientCookieSpec()).andReturn("standard").anyTimes();
Review Comment:
I'd use `org.apache.http.client.config.CookieSpecs.STANDARD` here.
##########
gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactory.java:
##########
@@ -369,4 +374,12 @@ private static long parseTimeout( String s ) {
Period p = Period.parse( s, f );
return p.toStandardDuration().getMillis();
}
+
+ private static String getCookieSpec(FilterConfig filterConfig) {
+ GatewayConfig globalConfig =
+
(GatewayConfig)filterConfig.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+ return globalConfig != null
Review Comment:
This will always return use the gateway-level value (very likely `null`)
even if `httpclient.cookieSpec` is set on the topology level. Based on the
other helper methods here (`getMaxConnections`, `getConnectionTimeout`,
`getSocketTimeout`,...) you should change this to check if
`httpclient.cookieSpec` is set in the topology and default to the GatewayConfig
value otherwise.
You may also want to refactor that code out to some private method to avoid
boilerplate private methods.
##########
gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java:
##########
@@ -1565,4 +1566,9 @@ public int getTokenMigrationProgressCount() {
return getInt(TOKEN_MIGRATION_PROGRESS_COUNT, 10);
}
+ @Override
+ public String getHttpClientCookieSpec() {
+ return get(HTTP_CLIENT_COOKIE_SPEC, null);
Review Comment:
The default value `null` is redundant here.
##########
gateway-spi/src/test/java/org/apache/knox/gateway/dispatch/DefaultHttpClientFactoryTest.java:
##########
@@ -246,6 +248,7 @@ public void testHttpRetriesValues() throws Exception {
expect(gatewayConfig.getHttpClientMaxConnections()).andReturn(32).anyTimes();
expect(gatewayConfig.getHttpClientConnectionTimeout()).andReturn(20000).anyTimes();
expect(gatewayConfig.getHttpClientSocketTimeout()).andReturn(20000).anyTimes();
+
expect(gatewayConfig.getHttpClientCookieSpec()).andReturn("standard").anyTimes();
Review Comment:
I'd use org.apache.http.client.config.CookieSpecs.STANDARD here.
Issue Time Tracking
-------------------
Worklog Id: (was: 905946)
Time Spent: 20m (was: 10m)
> Make http client cookie spec parameter configurable
> ---------------------------------------------------
>
> Key: KNOX-3007
> URL: https://issues.apache.org/jira/browse/KNOX-3007
> Project: Apache Knox
> Issue Type: Improvement
> Reporter: Attila Magyar
> Assignee: Attila Magyar
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The apache http client rejects cookies if the expiration date doesn't have
> the expected format (EEE, dd-MMM-yy HH:mm:ss z).
> {code}
> 2023-11-20 17:58:51,189 XXX WARN protocol.ResponseProcessCookies
> (ResponseProcessCookies.java:processCookies(130)) - Invalid cookie header:
> "Set-Cookie: sessionid=XXX; expires=Mon, 20 Nov 2023 23:03:51 GMT; HttpOnly;
> Max-Age=300; Path=/; SameSite=Lax; Secure". Invalid 'expires' attribute: Mon,
> 20 Nov 2023 23:03:51 GMT
> {code}
> This can be reconfigured by setting different cookiespec types:
> https://hc.apache.org/httpcomponents-client-4.5.x/current/httpclient/apidocs/org/apache/http/client/config/CookieSpecs.html
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
