Issue-281: Updates dependencies version to fix CVE-2023-2976 #282

a6dulaleem · 2023-09-21T07:29:00Z

Problem description
Upgrade dependencies in Schema registry which could have possible CVEs

Library	CVE Version
checkstyleToolVersion	10.12.3
protobufUtilVersion	3.24.3
guavaVersion	32.0.1-jre
swaggerJersey2JaxrsVersion	1.6.11

Problem location
gradle.properties

Suggestions for an improvement
upgrade lib versions to suggested versions to mitigate the CVEs.

Purpose of the change Fix #281

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

a6dulaleem · 2023-09-21T07:33:07Z

ran schema registry build 171 and it ran successful
ran flink connector and it got succeed build number is 420 all test passed.

shshashwat · 2023-09-21T08:42:37Z

ran schema registry build 171 and it ran successful ran flink connector and it got succeed build number is 420 all test passed.

Please create an issue and tag the PR with that

shshashwat · 2023-09-21T08:44:18Z

server/src/main/java/io/pravega/schemaregistry/storage/impl/group/InMemoryGroupTable.java

@@ -86,7 +86,7 @@ public CompletableFuture<Void> updateEntries(List<Entry<Integer>> updates) {
            TableKey key = update.getKey();
            Integer version = update.getVersion();
            Value<TableValue, Integer> val = table.get(key);
-            return version == null || (val != null && version.equals(val.getVersion()));
+            return version == null || val != null && version.equals(val.getVersion());


Didn't get the reason behind this change

As we are changing the Checkstyle version it was claimed by that, it was claiming that redundant Pair of parenthesis

shshashwat · 2023-09-21T08:44:52Z

server/src/main/java/io/pravega/schemaregistry/storage/StoreExceptions.java

     */
    public static StoreExceptions create(final Type type, final Throwable cause, final String errorMessage) {
-        Preconditions.checkArgument(cause != null || (errorMessage != null && !errorMessage.isEmpty()),
+        Preconditions.checkArgument(cause != null || errorMessage != null && !errorMessage.isEmpty(),


why is this change required?

As we are changing the Checkstyle version it was claimed by that, it was claiming that redundant Pair of parenthesis

shshashwat · 2023-09-21T08:45:29Z

...obuf/src/main/java/io/pravega/schemaregistry/serializer/protobuf/schemas/ProtobufSchema.java

@@ -153,6 +153,7 @@ public static <T extends GeneratedMessageV3> ProtobufSchema<GeneratedMessageV3>
     *
     * @param schemaInfo              Schema Info
     * @return {@link ProtobufSchema} with generic type {@link DynamicMessage} that captures protobuf schema.
+     * @throws IllegalArgumentException


method signature needs to be changed too?

As this method is throwing this exception, claimed by checkstylel.

if this method is throwing the exception, this should be added to the method signature as well. And how are we handling that exception?

Actually this method was already throwing IllegalArgumentException as this is a unchecked so it was not added in method signature.

Check style was claiming if we are not showing this in Doc.

This is a unchecked exception so doesn't require a throws clause, so why are we mentioning it in docs? Why check style is mandating to place this exception in docs ?

Checkstyle is mandating it because InvalidProtocolBufferException this exception is raising its throwing null pointer exception.
other I there is a way we can hide this from documentation by using hyphen after the exception, what it will do while generating doc it will remove this exception from the doc.

but i am not getting a way in which i just remove this throws from the method doc.

either checked or unchecked its being thrown by the method.

shshashwat · 2023-09-21T08:45:51Z

...alizers/json/src/main/java/io/pravega/schemaregistry/serializer/json/schemas/JSONSchema.java

@@ -74,7 +74,8 @@ private JSONSchema(SchemaInfo schemaInfo, String schemaString, Class<T> derived)
     *
     * @param tClass Class whose object's schema is used.
     * @param <T> Type of the Java class. 
-     * @return {@link JSONSchema} with generic type T that extracts and captures the json schema. 
+     * @return {@link JSONSchema} with generic type T that extracts and captures the json schema.
+     * @throws IllegalArgumentException


method signature needs to be changed too?

As this method is throwing this exception, claimed by checkstylel.

shshashwat · 2023-09-21T08:46:01Z

...alizers/json/src/main/java/io/pravega/schemaregistry/serializer/json/schemas/JSONSchema.java

@@ -96,7 +97,8 @@ public static <T> JSONSchema<T> of(Class<T> tClass) {
     * @param schema Schema to use. 
     * @param tClass class for the type of object
     * @param <T> Type of object
-     * @return Returns an JSONSchema with {@link Object} type. 
+     * @return Returns an JSONSchema with {@link Object} type.
+     * @throws IllegalArgumentException


same as above

claimed by checkstyle

shshashwat · 2023-09-21T08:46:10Z

...alizers/json/src/main/java/io/pravega/schemaregistry/serializer/json/schemas/JSONSchema.java

-     * @return Returns an JsonSchema of type T. 
+     * @param <T> Type of base class.
+     * @return Returns an JsonSchema of type T.
+     * @throws IllegalArgumentException


claimed by checkstyle

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

shshashwat · 2023-09-26T05:32:51Z

gradle.properties

@@ -39,10 +39,10 @@ nettyBoringSSLVersion=2.0.54.Final
 jacocoVersion=0.8.5
 protobufGradlePlugin=0.8.15
 protobufProtocVersion=3.21.7
-protobufUtilVersion=3.19.4
+protobufUtilVersion=3.24.3


I see there is a PR on Pravega too for similar issue. Can we check the versions and try to keep them in sync just in order to avoid backward-forward incompatibility issue at a later stage?

protobufUtilVersion is not being used in pravega.

protobufGradlePlugin=0.8.15 protobufProtocVersion=3.21.7

already syncing

munish1789 · 2023-09-26T05:56:49Z

checkstyle/checkstyle.xml

-      <property name="allowMissingJavadoc" value="true"/> <!--TODO: this should be enabled at one point. -->
-      <property name="ignoreMethodNamesRegex" value="^get.*$"/> <!--It would be nice if we could allow checking that the doc exists without also verifying the @returns is there also, but checkstyle does not allow that. -->
-      <property name="ignoreMethodNamesRegex" value="^has.*$"/> <!-- accessor for boolean e.g. hasVersion -->
-      <property name="suppressLoadErrors" value="false"/>


Why are these javaDocsMethod checkstyle properties removed?

These all are deprecated in new checkStyle version

Bhupender-Y · 2023-09-26T05:46:37Z

checkstyle/spotbugs-exclude.xml

+    <Match> <!-- fromBytes was not reporting this warning in earlier version of checkstyle -->
+        <Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE" />
+        <Method name="fromBytes" />
+    </Match>


Do new version force us to add this suppression ?

fromBytes method is returning some value from some map and that map we are already building on the top of the class you can see here

now there is no way that it could get NULL from the map

but The bugs still reported by newer version of the checkstyle.

and becuases of that we are suppressing this.

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

…6dulaleem/schema-registry into issue-281-update-guava-version

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

anishakj · 2023-09-26T11:32:41Z

server/src/main/java/io/pravega/schemaregistry/storage/impl/schemas/SchemaRecords.java

+        val versionSerializer = SERIALIZERS_BY_KEY_TYPE.get(keyClass);
+        if ( versionSerializer == null ) {
+            throw new SerializationException(String.format("No serializer found for the class %s", keyClass.toGenericString()));
+        }
        return (T) SERIALIZERS_BY_KEY_TYPE.get(keyClass).deserialize(bytes);


@a6dulaleem Can we use versionSerializer at line 430

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

anishakj

LGTM

a6dulaleem · 2023-09-26T12:14:23Z

...alizers/json/src/main/java/io/pravega/schemaregistry/serializer/json/schemas/JSONSchema.java

@@ -74,7 +74,7 @@ private JSONSchema(SchemaInfo schemaInfo, String schemaString, Class<T> derived)
     *
     * @param tClass Class whose object's schema is used.
     * @param <T> Type of the Java class. 
-     * @return {@link JSONSchema} with generic type T that extracts and captures the json schema. 
+     * @return {@link JSONSchema} with generic type T that extracts and captures the json schema.


Please Ignore these spaces are automatically removed by IntelliJ.

shshashwat

LGTM

Bhupender-Y

LGTM

a6dulaleem added 4 commits September 18, 2023 23:38

[Issue-281]: Changed guava version to fix CVE-2023-2976

2ae661f

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

[Issue-281]: Changed guava version to fix CVE-2023-2976

a9e8208

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

[issue-281] Changed guava, checkstyle version

61375e3

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

[issue-281]: Suppressing spotbugs warning for readbytes method

86fae3c

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

a6dulaleem marked this pull request as ready for review September 21, 2023 07:29

a6dulaleem requested a review from shshashwat September 21, 2023 07:33

anishakj requested a review from crazyzhou September 21, 2023 07:47

shshashwat reviewed Sep 21, 2023

View reviewed changes

shshashwat requested changes Sep 21, 2023

View reviewed changes

[issue-281]: update swagger Jersey version

90774c1

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

a6dulaleem requested review from shshashwat, munish1789 and Bhupender-Y September 21, 2023 13:59

shshashwat requested a review from abhinb September 25, 2023 11:10

Merge branch 'master' into issue-281-update-guava-version

e45cb58

shshashwat reviewed Sep 26, 2023

View reviewed changes

a6dulaleem requested a review from shshashwat September 26, 2023 05:44

munish1789 reviewed Sep 26, 2023

View reviewed changes

Bhupender-Y reviewed Sep 26, 2023

View reviewed changes

a6dulaleem added 4 commits September 26, 2023 03:27

[issue-281]: fixing spotbugs issue

4a6dc20

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

Merge branch 'issue-281-update-guava-version' of https://github.com/a…

1c91a8f

…6dulaleem/schema-registry into issue-281-update-guava-version

[issue-281] reverting Illegalstatement changes

d94f8bc

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

[issue-281] reverting Illegalstatement changes

0019d27

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

anishakj reviewed Sep 26, 2023

View reviewed changes

[issue-281] removing the extra spaces

0e2f76d

Signed-off-by: a6dulaleem <abdul.aleem1@dell.com>

a6dulaleem requested review from munish1789, Bhupender-Y and anishakj September 26, 2023 12:10

anishakj approved these changes Sep 26, 2023

View reviewed changes

a6dulaleem commented Sep 26, 2023

View reviewed changes

munish1789 approved these changes Sep 26, 2023

View reviewed changes

shshashwat approved these changes Sep 26, 2023

View reviewed changes

Bhupender-Y approved these changes Sep 27, 2023

View reviewed changes

a6dulaleem changed the title ~~Issue-281: Updates guava version to fix CVE-2023-2976~~ Issue-281: Updates dependencies version to fix CVE-2023-2976 Sep 27, 2023

shshashwat merged commit c4b6a43 into pravega:master Sep 27, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue-281: Updates dependencies version to fix CVE-2023-2976 #282

Issue-281: Updates dependencies version to fix CVE-2023-2976 #282

a6dulaleem commented Sep 21, 2023 •

edited

Loading

a6dulaleem commented Sep 21, 2023

shshashwat commented Sep 21, 2023

shshashwat Sep 21, 2023

a6dulaleem Sep 21, 2023

shshashwat Sep 21, 2023

a6dulaleem Sep 21, 2023

shshashwat Sep 21, 2023

a6dulaleem Sep 21, 2023

shshashwat Sep 22, 2023

a6dulaleem Sep 25, 2023

munish1789 Sep 26, 2023

a6dulaleem Sep 26, 2023

shshashwat Sep 21, 2023

a6dulaleem Sep 21, 2023

shshashwat Sep 21, 2023

a6dulaleem Sep 21, 2023

shshashwat Sep 21, 2023

a6dulaleem Sep 21, 2023

shshashwat Sep 26, 2023

a6dulaleem Sep 26, 2023

a6dulaleem Sep 26, 2023

munish1789 Sep 26, 2023

a6dulaleem Sep 26, 2023 •

edited

Loading

Bhupender-Y Sep 26, 2023

a6dulaleem Sep 26, 2023

anishakj Sep 26, 2023

a6dulaleem Sep 26, 2023

anishakj left a comment

a6dulaleem Sep 26, 2023 •

edited

Loading

shshashwat left a comment

Bhupender-Y left a comment

Issue-281: Updates dependencies version to fix CVE-2023-2976 #282

Issue-281: Updates dependencies version to fix CVE-2023-2976 #282

Conversation

a6dulaleem commented Sep 21, 2023 • edited Loading

a6dulaleem commented Sep 21, 2023

shshashwat commented Sep 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a6dulaleem Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anishakj left a comment

Choose a reason for hiding this comment

a6dulaleem Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

shshashwat left a comment

Choose a reason for hiding this comment

Bhupender-Y left a comment

Choose a reason for hiding this comment

a6dulaleem commented Sep 21, 2023 •

edited

Loading

a6dulaleem Sep 26, 2023 •

edited

Loading

a6dulaleem Sep 26, 2023 •

edited

Loading