[
https://issues.apache.org/jira/browse/GEODE-8217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143243#comment-17143243
]
ASF GitHub Bot commented on GEODE-8217:
---------------------------------------
DonalEvans commented on a change in pull request #5256:
URL: https://github.com/apache/geode/pull/5256#discussion_r444434215
##########
File path:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManagerConfiguration.java
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.modules.session.catalina;
+
+/**
+ * Method used by Catalina XML configuration.
+ */
+@SuppressWarnings("unused")
+interface DeltaSessionManagerConfiguration {
+
+ void setRegionName(String regionName);
+
+ String getRegionName();
+
+ void setEnableLocalCache(boolean enableLocalCache);
+
+ boolean getEnableLocalCache();
+
+ void setMaxActiveSessions(int maxActiveSessions);
+
+ int getMaxActiveSessions();
+
+ void setRegionAttributesId(String regionType);
+
+ String getRegionAttributesId();
+
+ void setEnableGatewayDeltaReplication(boolean enableGatewayDeltaReplication);
+
+ boolean getEnableGatewayDeltaReplication();
+
+ void setEnableGatewayReplication(boolean enableGatewayReplication);
+
+ boolean getEnableGatewayReplication();
+
+ void setEnableDebugListener(boolean enableDebugListener);
+
+ boolean getEnableDebugListener();
+
+ boolean isCommitValveEnabled();
+
+ void setEnableCommitValve(boolean enable);
+
+ boolean isCommitValveFailfastEnabled();
+
+ void setEnableCommitValveFailfast(boolean enable);
+
+ boolean isBackingCacheAvailable();
+
+ /**
+ * @deprecated No replacement. Always refer deserialized form.
+ */
+ @Deprecated
+ void setPreferDeserializedForm(boolean enable);
+
+ /**
+ * @deprecated No replacement. Always refer deserialized form.
Review comment:
Couple of typos here. Should be "Always prefer deserialized form."
##########
File path:
extensions/geode-modules-tomcat7/src/test/java/org/apache/geode/modules/session/catalina/DeltaSession7Test.java
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.modules.session.catalina;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionBindingEvent;
+
+import org.apache.catalina.Context;
+import org.apache.juli.logging.Log;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.internal.util.BlobHelper;
+
+public class DeltaSession7Test extends AbstractDeltaSessionTest {
+ final HttpSessionAttributeListener listener =
mock(HttpSessionAttributeListener.class);
+
+ @Before
+ @Override
+ public void setup() {
+ super.setup();
+
+ final Context context = mock(Context.class);
+ when(manager.getContainer()).thenReturn(context);
+ when(context.getApplicationEventListeners()).thenReturn(new Object[]
{listener});
+ when(context.getLogger()).thenReturn(mock(Log.class));
+ }
+
+ @Test
+ public void serializedAttributesNotLeakedInAttributeReplaceEvent() throws
IOException {
+ final DeltaSession7 session = spy(new DeltaSession7(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ final Object value2 = "value2";
+ session.setAttribute(name, value2);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeReplaced(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isEqualTo(value1);
+ }
+
+ @Test
+ public void serializedAttributesNotLeakedInAttributeRemovedEvent() throws
IOException {
+ final DeltaSession7 session = spy(new DeltaSession7(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ session.removeAttribute(name);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeRemoved(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isEqualTo(value1);
+ }
+
+ @Test
+ public void
serializedAttributesLeakedInAttributeReplaceEventWhenPreferSerializedFormFalse()
+ throws IOException {
+ setPreferSeserializedForm();
+
+ final DeltaSession7 session = spy(new DeltaSession7(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ final Object value2 = "value2";
+ session.setAttribute(name, value2);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeReplaced(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+ }
+
+ @Test
+ public void
serializedAttributesLeakedInAttributeRemovedEventWhenPreferSerializedFormFalse()
+ throws IOException {
+ setPreferSeserializedForm();
+
+ final DeltaSession7 session = spy(new DeltaSession7(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ session.removeAttribute(name);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeRemoved(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+ }
+
+ @SuppressWarnings("deprecation")
+ protected void setPreferSeserializedForm() {
Review comment:
Typo here, should be "setPreferDeserializedForm". Also, tests using this
method should probably be renamed, since they state the condition
"WhenPreferSerializedFormFalse" rather than "WhenPreferDeserializedFormFalse".
It's also a little misleading as a method name, since
`setPreferDeserializedForm()` with no argument would tend to imply setting the
value to `true` rather than `false`.
##########
File path:
extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/catalina/DeltaSession8Test.java
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.modules.session.catalina;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionBindingEvent;
+
+import org.apache.catalina.Context;
+import org.apache.juli.logging.Log;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.internal.util.BlobHelper;
+
+public class DeltaSession8Test extends AbstractDeltaSessionTest {
+ final HttpSessionAttributeListener listener =
mock(HttpSessionAttributeListener.class);
+
+ @Before
+ @Override
+ public void setup() {
+ super.setup();
+
+ final Context context = mock(Context.class);
+ when(manager.getContext()).thenReturn(context);
+ when(context.getApplicationEventListeners()).thenReturn(new Object[]
{listener});
+ when(context.getLogger()).thenReturn(mock(Log.class));
+ }
+
+ @Test
+ public void serializedAttributesNotLeakedInAttributeReplaceEvent() throws
IOException {
+ final DeltaSession8 session = spy(new DeltaSession8(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ final Object value2 = "value2";
+ session.setAttribute(name, value2);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeReplaced(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isEqualTo(value1);
+ }
+
+ @Test
+ public void serializedAttributesNotLeakedInAttributeRemovedEvent() throws
IOException {
+ final DeltaSession8 session = spy(new DeltaSession8(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ session.removeAttribute(name);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeRemoved(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isEqualTo(value1);
+ }
+
+ @Test
+ public void
serializedAttributesLeakedInAttributeReplaceEventWhenPreferSerializedFormFalse()
+ throws IOException {
+ setPreferSeserializedForm();
+
+ final DeltaSession8 session = spy(new DeltaSession8(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ final Object value2 = "value2";
+ session.setAttribute(name, value2);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeReplaced(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+ }
+
+ @Test
+ public void
serializedAttributesLeakedInAttributeRemovedEventWhenPreferSerializedFormFalse()
+ throws IOException {
+ setPreferSeserializedForm();
+
+ final DeltaSession8 session = spy(new DeltaSession8(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ session.removeAttribute(name);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeRemoved(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+ }
+
+ @SuppressWarnings("deprecation")
+ protected void setPreferSeserializedForm() {
Review comment:
See comment on DeltaSession7Test.java regarding this typo and method
names.
##########
File path:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
##########
@@ -328,14 +353,14 @@ Object getAttributeWithoutDeserialize(String name) {
public void invalidate() {
super.invalidate();
// getOperatingRegion().destroy(this.id, true); // already done in super
(remove)
Review comment:
Can this commented code be removed?
##########
File path:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
##########
@@ -69,17 +71,21 @@
private final transient Object changeLock = new Object();
- private final List<DeltaSessionAttributeEvent> eventQueue = new
ArrayList<>();
+ private final ArrayList<DeltaSessionAttributeEvent> eventQueue = new
ArrayList<>();
private transient GatewayDeltaEvent currentGatewayDeltaEvent;
private transient boolean expired = false;
+ /**
+ * @deprecated No replacement. Always refer deserialized form.
Review comment:
Typo here, should be "Always prefer deserialized form."
##########
File path:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManager.java
##########
@@ -107,6 +110,10 @@
private static final boolean DEFAULT_ENABLE_COMMIT_VALVE_FAILFAST = false;
+ /**
+ * @deprecated No replacement. Always refer deserialized form.
Review comment:
Typo here, should be "Always prefer deserialized form."
##########
File path:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManager.java
##########
@@ -130,6 +137,10 @@
private boolean enableDebugListener = DEFAULT_ENABLE_DEBUG_LISTENER;
+ /**
+ * @deprecated No replacement. Always refer deserialized form.
Review comment:
Typo here, should be "Always prefer deserialized form."
##########
File path:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManager.java
##########
@@ -213,64 +225,78 @@ public boolean getEnableGatewayDeltaReplication() {
return false; // disabled
}
- @SuppressWarnings("unused")
+ @Override
public void setEnableGatewayDeltaReplication(boolean
enableGatewayDeltaReplication) {
// this.enableGatewayDeltaReplication = enableGatewayDeltaReplication;
// Disabled. Keeping the method for backward compatibility.
}
@Override
public boolean getEnableGatewayReplication() {
- return this.enableGatewayReplication;
+ return enableGatewayReplication;
}
- @SuppressWarnings("unused")
+ @Override
public void setEnableGatewayReplication(boolean enableGatewayReplication) {
this.enableGatewayReplication = enableGatewayReplication;
}
@Override
public boolean getEnableDebugListener() {
- return this.enableDebugListener;
+ return enableDebugListener;
}
- @SuppressWarnings("unused")
+ @Override
public void setEnableDebugListener(boolean enableDebugListener) {
this.enableDebugListener = enableDebugListener;
}
@Override
public boolean isCommitValveEnabled() {
- return this.enableCommitValve;
+ return enableCommitValve;
}
+ @Override
public void setEnableCommitValve(boolean enable) {
- this.enableCommitValve = enable;
+ enableCommitValve = enable;
}
@Override
public boolean isCommitValveFailfastEnabled() {
- return this.enableCommitValveFailfast;
+ return enableCommitValveFailfast;
}
- @SuppressWarnings("unused")
+ @Override
public void setEnableCommitValveFailfast(boolean enable) {
- this.enableCommitValveFailfast = enable;
+ enableCommitValveFailfast = enable;
}
@Override
public boolean isBackingCacheAvailable() {
return sessionCache.isBackingCacheAvailable();
}
- @SuppressWarnings("unused")
+ /**
+ * @deprecated No replacement. Always refer deserialized form.
Review comment:
Typo here, should be "Always prefer deserialized form."
##########
File path:
extensions/geode-modules-tomcat9/src/test/java/org/apache/geode/modules/session/catalina/DeltaSession9Test.java
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.modules.session.catalina;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionBindingEvent;
+
+import org.apache.catalina.Context;
+import org.apache.juli.logging.Log;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.internal.util.BlobHelper;
+
+public class DeltaSession9Test extends AbstractDeltaSessionTest {
+ final HttpSessionAttributeListener listener =
mock(HttpSessionAttributeListener.class);
+
+ @Before
+ @Override
+ public void setup() {
+ super.setup();
+
+ final Context context = mock(Context.class);
+ when(manager.getContext()).thenReturn(context);
+ when(context.getApplicationEventListeners()).thenReturn(new Object[]
{listener});
+ when(context.getLogger()).thenReturn(mock(Log.class));
+ }
+
+ @Test
+ public void serializedAttributesNotLeakedInAttributeReplaceEvent() throws
IOException {
+ final DeltaSession9 session = spy(new DeltaSession9(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ final Object value2 = "value2";
+ session.setAttribute(name, value2);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeReplaced(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isEqualTo(value1);
+ }
+
+ @Test
+ public void serializedAttributesNotLeakedInAttributeRemovedEvent() throws
IOException {
+ final DeltaSession9 session = spy(new DeltaSession9(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ session.removeAttribute(name);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeRemoved(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isEqualTo(value1);
+ }
+
+ @Test
+ public void
serializedAttributesLeakedInAttributeReplaceEventWhenPreferSerializedFormFalse()
+ throws IOException {
+ setPreferSeserializedForm();
+
+ final DeltaSession9 session = spy(new DeltaSession9(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ final Object value2 = "value2";
+ session.setAttribute(name, value2);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeReplaced(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+ }
+
+ @Test
+ public void
serializedAttributesLeakedInAttributeRemovedEventWhenPreferSerializedFormFalse()
+ throws IOException {
+ setPreferSeserializedForm();
+
+ final DeltaSession9 session = spy(new DeltaSession9(manager));
+ session.setValid(true);
+ final String name = "attribute";
+ final Object value1 = "value1";
+ final byte[] serializedValue1 = BlobHelper.serializeToBlob(value1);
+ // simulates initial deserialized state with serialized attribute values.
+ session.getAttributes().put(name, serializedValue1);
+
+ session.removeAttribute(name);
+
+ final ArgumentCaptor<HttpSessionBindingEvent> event =
+ ArgumentCaptor.forClass(HttpSessionBindingEvent.class);
+ verify(listener).attributeRemoved(event.capture());
+ verifyNoMoreInteractions(listener);
+ assertThat(event.getValue().getValue()).isInstanceOf(byte[].class);
+ }
+
+ @SuppressWarnings("deprecation")
+ protected void setPreferSeserializedForm() {
Review comment:
See comment on DeltaSession7Test.java regarding this typo and method
names.
##########
File path:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSessionManager.java
##########
@@ -213,64 +225,78 @@ public boolean getEnableGatewayDeltaReplication() {
return false; // disabled
}
- @SuppressWarnings("unused")
+ @Override
public void setEnableGatewayDeltaReplication(boolean
enableGatewayDeltaReplication) {
// this.enableGatewayDeltaReplication = enableGatewayDeltaReplication;
// Disabled. Keeping the method for backward compatibility.
}
@Override
public boolean getEnableGatewayReplication() {
- return this.enableGatewayReplication;
+ return enableGatewayReplication;
}
- @SuppressWarnings("unused")
+ @Override
public void setEnableGatewayReplication(boolean enableGatewayReplication) {
this.enableGatewayReplication = enableGatewayReplication;
}
@Override
public boolean getEnableDebugListener() {
- return this.enableDebugListener;
+ return enableDebugListener;
}
- @SuppressWarnings("unused")
+ @Override
public void setEnableDebugListener(boolean enableDebugListener) {
this.enableDebugListener = enableDebugListener;
}
@Override
public boolean isCommitValveEnabled() {
- return this.enableCommitValve;
+ return enableCommitValve;
}
+ @Override
public void setEnableCommitValve(boolean enable) {
- this.enableCommitValve = enable;
+ enableCommitValve = enable;
}
@Override
public boolean isCommitValveFailfastEnabled() {
- return this.enableCommitValveFailfast;
+ return enableCommitValveFailfast;
}
- @SuppressWarnings("unused")
+ @Override
public void setEnableCommitValveFailfast(boolean enable) {
- this.enableCommitValveFailfast = enable;
+ enableCommitValveFailfast = enable;
}
@Override
public boolean isBackingCacheAvailable() {
return sessionCache.isBackingCacheAvailable();
}
- @SuppressWarnings("unused")
+ /**
+ * @deprecated No replacement. Always refer deserialized form.
+ */
+ @Deprecated
+ @Override
public void setPreferDeserializedForm(boolean enable) {
- this.preferDeserializedForm = enable;
+ log.warn("Use of deprecated preferDeserializedForm property to be removed
in future release.");
+ if (!enable) {
+ log.warn(
+ "Use of HttpSessionAttributeListener may result in serialized form
in HttpSessionBindingEvent.");
+ }
+ preferDeserializedForm = enable;
}
+ /**
+ * @deprecated No replacement. Always refer deserialized form.
Review comment:
Typo here, should be "Always prefer deserialized form."
##########
File path:
extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java
##########
@@ -616,7 +642,7 @@ public int getSizeInBytes() {
return size;
}
- @SuppressWarnings({"unchecked", "rawtypes"})
+ @SuppressWarnings({"unchecked"})
Review comment:
While we're here, it might be worth cleaning up uses of this warning
suppression using the `uncheckedCast()` method, as used elsewhere in this
change set. There is one cast in this method (line 651) and one on line 633 of
`getSizeInBytes()`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Geode session replication could leak internal serialized bytes during
> HttpSessionAttributeListener invocation even when preferDeserializedForm is
> set to true
> -------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GEODE-8217
> URL: https://issues.apache.org/jira/browse/GEODE-8217
> Project: Geode
> Issue Type: Bug
> Components: http session
> Reporter: Eric Shu
> Assignee: Eric Shu
> Priority: Major
> Labels: caching-applications
>
> When preferDeserializedForm is set to true (default value), session object
> should not contain serialized byte in the cache. However, the following
> exception shows that product leaks the serialized bytes.
> {noformat}
> Jun 02, 2020 3:31:58 PM org.apache.catalina.session.StandardSession
> setAttribute
> SEVERE: Session attribute event listener threw exception
> java.lang.ClassCastException: [B cannot be cast to java.lang.String
> at
> org.apache.geode.modules.session.AccessAttributeValueListener.attributeReplaced(AccessAttributeValueListener.java:34)
> at
> org.apache.catalina.session.StandardSession.setAttribute(StandardSession.java:1482)
> at
> org.apache.geode.modules.session.catalina.DeltaSession.setAttribute(DeltaSession.java:262)
> at
> org.apache.catalina.session.StandardSession.setAttribute(StandardSession.java:1385)
> at
> org.apache.catalina.session.StandardSessionFacade.setAttribute(StandardSessionFacade.java:137)
> at
> org.apache.geode.modules.session.catalina.DeltaSessionFacade.setAttribute(DeltaSessionFacade.java:49)
> at
> org.apache.geode.modules.session.CommandServlet.doGet(CommandServlet.java:64)
> at javax.servlet.http.HttpServlet.service(HttpServlet.java:634)
> at javax.servlet.http.HttpServlet.service(HttpServlet.java:741)
> at
> org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
> at
> org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
> at
> org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
> at
> org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
> at
> org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
> at
> org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:199)
> at
> org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96)
> at
> org.apache.geode.modules.session.catalina.CommitSessionValve.invoke(CommitSessionValve.java:47)
> at
> org.apache.geode.modules.session.catalina.JvmRouteBinderValve.invoke(JvmRouteBinderValve.java:45)
> at
> org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:543)
> at
> org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:139)
> at
> org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:81)
> at
> org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:678)
> at
> org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
> at
> org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
> at
> org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:609)
> at
> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
> at
> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:810)
> at
> org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1623)
> at
> org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> at
> org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> at java.lang.Thread.run(Thread.java:748)
> {noformat}
> Please note if preferDeserializedForm is set to false, this issue could still
> exist, unless HttpSessionBindingEvent.getValue() is not being accessed by the
> application. Otherwise, user should set preferDeserializedForm to true to
> avoid this issue.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
