Skip to content

Conversation

@robert-s-ubi
Copy link
Contributor

Description

The per-message compression extension was badly broken, leading to exceptions and loss of communication between client and server.

Fix the extension negotiation to correctly send the current state of the client_no_context_takeover and server_no_context_takeover configuration variables, and ORing the received extensions with the received ones, to ensure client and server end up with the same configuration.

Fix the application of these configuration variables, which must be swapped on the client vs. the server: The configuration of the deflater on the client side must affect the inflater on the server side, and vice versa.

Remove the requestedParameters map as there is no need for it.

Related Issue

This fixes issue #1496

Motivation and Context

The broken extension negotiation and application of configuration made it impossible to use the compression extension WITH context takeover, which is the most efficient compression, and the only one which makes sense for applications using smaller message sizes, like OCPP.

With this fix, the Java-OCA-OCPP library can use efficient compression for OCPP messages.

How Has This Been Tested?

Tested within the Java-OCA-OCPP library, and an integration test which creates an OCPP client and server and connects these on the local machine. When enabling WebSocket compression with the existing library, the test always crashed with an Exception at the second message exchange, due to the inflater and deflater resets being incorrectly applied.

With this fix, the tests which crashed before all pass.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

The per-message compression extension was badly broken, leading to
exceptions and loss of communication between client and server.

Fix the extension negotiation to correctly send the current state of the
client_no_context_takeover and server_no_context_takeover configuration
variables, and ORing the received extensions with the received ones, to
ensure client and server end up with the same configuration.

Fix the application of these configuration variables, which must be
swapped on the client vs. the server: The configuration of the deflater
on the client side must affect the inflater on the server side, and vice
versa.

Remove the requestedParameters map as there is no need for it.

This fixes issue TooTallNate#1496
@robert-s-ubi
Copy link
Contributor Author

@marci4 since you last fixed this class, would you maybe have the time to review and approve this PR?

Resetting the inflater/deflater is never a "must". Only _not_ resetting
the inflater is a "must" if the deflater is not being reset.
@robert-s-ubi
Copy link
Contributor Author

The failing CI run seems to be a server issue. I cannot find a button to rerun it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant