Skip to content

Fix ContentTypeId conflict #3967

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.

Sign up for GitHub

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

Merged
beiwei30 merged 5 commits into from
May 5, 2019
Merged

Fix ContentTypeId conflict #3967

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
820021b
fix ContentTypeId conflict
CrazyHZM May 1, 2019
c47240c
Merge branch 'master' into avroSerializationId
CrazyHZM May 1, 2019
3197667
modify
CrazyHZM May 1, 2019
2022be7
Merge branch 'master' into avroSerializationId
CrazyHZM May 5, 2019
e0d5c00
modify avro id
CrazyHZM May 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions dubbo-common/src/main/java/org/apache/dubbo/common/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,4 +875,18 @@ public class Constants {
public static final String METRICS_PROTOCOL = "metrics.protocol";


/**
* Serizlization ContentTypeId
*/
public static final byte AVRO_SERIALIZATION_ID = 1;
Copy link
Copy Markdown
Member

@beiwei30 beiwei30 May 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is wrong, avro is 10...

Copy link
Copy Markdown
Member

@beiwei30 beiwei30 May 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public static final byte AVRO_SERIALIZATION_ID = 11 should be better

Copy link
Copy Markdown
Member Author

@CrazyHZM CrazyHZM May 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public static final byte AVRO_SERIALIZATION_ID = 11 should be better

What are the uses of 1 and 5 here?

Copy link
Copy Markdown
Member

@beiwei30 beiwei30 May 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's simply leave it as is. I believe the original intention is: 1, 2, 4, 8, 16...

Copy link
Copy Markdown
Member Author

@CrazyHZM CrazyHZM May 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have modified it.

public static final byte HESSIAN2_SERIALIZATION_ID = 2;
public static final byte JAVA_SERIALIZATION_ID = 3;
public static final byte COMPACTED_JAVA_SERIALIZATION_ID = 4;
public static final byte FASTJSON_SERIALIZATION_ID = 6;
public static final byte NATIVE_JAVA_SERIALIZATION_ID = 7;
public static final byte KRYO_SERIALIZATION_ID = 8;
public static final byte FST_SERIALIZATION_ID = 9;
public static final byte PROTOSTUFF_SERIALIZATION_ID = 10;
public static final byte GSON_SERIALIZATION_ID = 16;

Copy link
Copy Markdown
Contributor

@cvictory cvictory May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can define a sub-constant class. What do you think?

Copy link
Copy Markdown
Contributor

@kun-song kun-song May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for a specific constants class

Copy link
Copy Markdown
Member Author

@CrazyHZM CrazyHZM May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this suggestion is good, I will modify it later.

}
3 changes: 2 additions & 1 deletion ...ng/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/CodecSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ public static Serialization getSerialization(URL url, Byte id) throws IOExceptio
String serializationName = url.getParameter(Constants.SERIALIZATION_KEY, Constants.DEFAULT_REMOTING_SERIALIZATION);
// Check if "serialization id" passed from network matches the id on this side(only take effect for JDK serialization), for security purpose.
if (serialization == null
|| ((id == 3 || id == 7 || id == 4) && !(serializationName.equals(ID_SERIALIZATIONNAME_MAP.get(id))))) {
|| ((id == Constants.JAVA_SERIALIZATION_ID || id == Constants.NATIVE_JAVA_SERIALIZATION_ID || id == Constants.COMPACTED_JAVA_SERIALIZATION_ID)
&& !(serializationName.equals(ID_SERIALIZATIONNAME_MAP.get(id))))) {
throw new IOException("Unexpected serialization id:" + id + " received from network, please check if the peer send the right id.");
}
return serialization;
Expand Down
3 changes: 2 additions & 1 deletion ...lization-avro/src/main/java/org/apache/dubbo/common/serialize/avro/AvroSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.dubbo.common.serialize.avro;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -29,7 +30,7 @@ public class AvroSerialization implements Serialization {

@Override
public byte getContentTypeId() {
return 10;
return Constants.AVRO_SERIALIZATION_ID;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...tjson/src/main/java/org/apache/dubbo/common/serialize/fastjson/FastJsonSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.dubbo.common.serialize.fastjson;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -36,7 +37,7 @@ public class FastJsonSerialization implements Serialization {

@Override
public byte getContentTypeId() {
return 6;
return Constants.FASTJSON_SERIALIZATION_ID;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...rialization-fst/src/main/java/org/apache/dubbo/common/serialize/fst/FstSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.dubbo.common.serialize.fst;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -36,7 +37,7 @@ public class FstSerialization implements Serialization {

@Override
public byte getContentTypeId() {
return 9;
return Constants.FST_SERIALIZATION_ID;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...lization-gson/src/main/java/org/apache/dubbo/common/serialize/gson/GsonSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.dubbo.common.serialize.gson;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -31,7 +32,7 @@ public class GsonSerialization implements Serialization {

@Override
public byte getContentTypeId() {
return 16;
return Constants.GSON_SERIALIZATION_ID;
}

@Override
Expand Down
5 changes: 2 additions & 3 deletions ...sian2/src/main/java/org/apache/dubbo/common/serialize/hessian2/Hessian2Serialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.dubbo.common.serialize.hessian2;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -34,11 +35,9 @@
*/
public class Hessian2Serialization implements Serialization {

public static final byte ID = 2;

@Override
public byte getContentTypeId() {
return ID;
return Constants.HESSIAN2_SERIALIZATION_ID;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...-jdk/src/main/java/org/apache/dubbo/common/serialize/java/CompactedJavaSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.dubbo.common.serialize.java;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -36,7 +37,7 @@ public class CompactedJavaSerialization implements Serialization {

@Override
public byte getContentTypeId() {
return 4;
return Constants.COMPACTED_JAVA_SERIALIZATION_ID;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...alization-jdk/src/main/java/org/apache/dubbo/common/serialize/java/JavaSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.dubbo.common.serialize.java;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -36,7 +37,7 @@ public class JavaSerialization implements Serialization {

@Override
public byte getContentTypeId() {
return 3;
return Constants.JAVA_SERIALIZATION_ID;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions ...k/src/main/java/org/apache/dubbo/common/serialize/nativejava/NativeJavaSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.dubbo.common.serialize.nativejava;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -35,11 +36,10 @@
*/
public class NativeJavaSerialization implements Serialization {

public static final String NAME = "nativejava";

@Override
public byte getContentTypeId() {
return 7;
return Constants.NATIVE_JAVA_SERIALIZATION_ID;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...lization-kryo/src/main/java/org/apache/dubbo/common/serialize/kryo/KryoSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.dubbo.common.serialize.kryo;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -36,7 +37,7 @@ public class KryoSerialization implements Serialization {

@Override
public byte getContentTypeId() {
return 8;
return Constants.KRYO_SERIALIZATION_ID;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...f/src/main/java/org/apache/dubbo/common/serialize/protostuff/ProtostuffSerialization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.apache.dubbo.common.serialize.protostuff;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
Expand All @@ -36,7 +37,7 @@
public class ProtostuffSerialization implements Serialization {
@Override
public byte getContentTypeId() {
return 10;
return Constants.PROTOSTUFF_SERIALIZATION_ID;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...tion-test/src/test/java/org/apache/dubbo/common/serialize/avro/AvroSerializationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.dubbo.common.serialize.avro;

import org.apache.dubbo.common.Constants;
import org.apache.dubbo.common.serialize.ObjectInput;
import org.apache.dubbo.common.serialize.ObjectOutput;
import org.hamcrest.Matchers;
Expand Down Expand Up @@ -46,7 +47,7 @@ public void testContentType() {

@Test
public void testContentTypeId() {
assertThat(avroSerialization.getContentTypeId(), is((byte) 10));
assertThat(avroSerialization.getContentTypeId(), is(Constants.AVRO_SERIALIZATION_ID));
}

@Test
Expand Down
⚡