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

[bugfix] Remove invalid marked transient on cacheKey as reported by sonarlint #1084

Merged
merged 2 commits into from Aug 21, 2017

Conversation

Projects
None yet
3 participants
@hazendaz
Member

hazendaz commented Aug 21, 2017

Test cases written around this to avoid this serious flaw in the future,
configured origin of scanning asking for transient to be added, added
comment about when this would actually fail serialization, and wrote
test for that as well.


Note: revert #989

[bugfix] Remove invalid marked transient on cacheKey as reported by s…
…onarlint

Test cases written around this to avoid this serious flaw in the future,
configured origin of scanning asking for transient to be added, added
comment about when this would actually fail serialization, and wrote
test for that as well.

@hazendaz hazendaz requested a review from harawata Aug 21, 2017

@hazendaz

This comment has been minimized.

Member

hazendaz commented Aug 21, 2017

#989 issue is invalid given bulk of use-case. The fix actually made things worse. While a real non serializable issue would get that error, the general valid use case will not get null on underlying object list defeating purpose of the class. Test case provided shows both bad case that is not serializable and valid case that is.

@hazendaz

This comment has been minimized.

Member

hazendaz commented Aug 21, 2017

hang on on this...fixing mistakenly added commit.

@hazendaz

This comment has been minimized.

Member

hazendaz commented Aug 21, 2017

OK - ready to go.

@hazendaz

This comment has been minimized.

Member

hazendaz commented Aug 21, 2017

Note: Attempting to make updateList transient again will cause a failure in both unit tests as null pointer exceptions. This is expected behaviour and will prevent accidently doing this in the future.

}
private void canSerialize(final CacheKey object) throws ClassNotFoundException, IOException {
FileOutputStream fout = new FileOutputStream("target/address.ser");

This comment has been minimized.

@FrantaM

FrantaM Aug 21, 2017

Contributor

No need to use files - ByteArrayOutputStream and ByteArrayInputStream will do just fine.

This comment has been minimized.

@hazendaz

hazendaz Aug 21, 2017

Member

Can you show me how to do that and read the data back? I was using ByteArrayOutputStream but then wanted to read it back so changed to files.

This comment has been minimized.

@FrantaM

FrantaM Aug 21, 2017

Contributor
private static <T> T serialize(T object) throws Exception {
    ByteArrayOutputStream baos = new ByteArrayOutputStream();
    new ObjectOutputStream(baos).writeObject(object);
 
    ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
    return (T) new ObjectInputStream(bais).readObject();
}
@hazendaz

This comment has been minimized.

Member

hazendaz commented Aug 21, 2017

@FrantaM Merging now, will look at any comments you have back or re-view later when I have some more time. This for now will get master back in working state for you. Thanks.

@hazendaz hazendaz merged commit 9ffabe2 into mybatis:master Aug 21, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hazendaz

This comment has been minimized.

Member

hazendaz commented Aug 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment