Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java serialization compression bug #12104

Closed
sancar opened this issue Jan 8, 2018 · 3 comments
Closed

Java serialization compression bug #12104

sancar opened this issue Jan 8, 2018 · 3 comments

Comments

@sancar
Copy link
Member

@sancar sancar commented Jan 8, 2018

We have a bug when following configuration is applied.

SerializationConfig.setEnableCompression(true);

Since GZipInputStream read more bytes than it needs, the fields coming after a compressed data is not able to read.
Following reproduces the issue:

 @Test
    public void testCompression_withArrayList() {
        DefaultSerializationServiceBuilder defaultSerializationServiceBuilder = new DefaultSerializationServiceBuilder();
        SerializationService ss = defaultSerializationServiceBuilder.setEnableCompression(true).build();

        ArrayList<SampleSerializable> expected = new ArrayList<SampleSerializable>();
        for (int i = 0; i < 10; i++) {
            expected.add(new SampleSerializable(i));
        }
        Data data = ss.toData(expected);
        ArrayList<SampleSerializable> result = ss.toObject(data);

        assertEquals(expected, result);
    }

If any of our internal operations or users use ObjectDataOutput.writeObject(x) with a java serializable, issue appears.

A fix is available as follows but requires one more copy when writing the object. Also relies on an internal value( Gzip Header Size ).
#12099

One more suggestion is that we could completely remove compression feature. Arguments for that:

  1. We will remove the bug.
  2. compression is often misunderstood. We have seen that users expects that all data is compressed when it is used. But It only applies to Java Serializable's.
  3. Java Serializable is not used much by our users. They either their own serialization methods or ours (Portable/IdentifiedDataSerializable).
@sancar sancar added this to the 3.9.3 milestone Jan 8, 2018
@sancar
Copy link
Member Author

@sancar sancar commented Jan 8, 2018

This bug is found when investigating following issue
hazelcast/hazelcast-enterprise#1750

@sancar
Copy link
Member Author

@sancar sancar commented Jan 8, 2018

One other concern is rolling upgrade:
With both of the following fixes we are breaking rolling upgrade.

  1. #12099 with extra length field
  2. removing compression feature.

I will spend some time to figure out a 3rd way to not to break rolling upgrade.

@pveentjer
Copy link
Member

@pveentjer pveentjer commented Jan 9, 2018

It is indeed very (very) strange that it only applies to java serializable. The name certainly is misleading.

sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
sancar added a commit to sancar/hazelcast that referenced this issue Jan 9, 2018
Gzip compression reads bytes ahead. That was causing serialization
bugs when more data available after a object serialized with
compression enabled.

As fix, we are rewinding the stream to correct position so that
next data in the stream can be read correctly.
Thanks @jerrinot for the fix.

fixes hazelcast/hazelcast-enterprise#1750
fixes hazelcast#12104
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.