Fix Per-Message Compression Extension support#1497
Closed
robert-s-ubi wants to merge 4 commits intoTooTallNate:masterfrom
Closed
Fix Per-Message Compression Extension support#1497robert-s-ubi wants to merge 4 commits intoTooTallNate:masterfrom
robert-s-ubi wants to merge 4 commits intoTooTallNate:masterfrom
Conversation
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
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.
This was referenced Dec 19, 2025
Contributor
Author
|
The failing CI run seems to be a server issue. I cannot find a button to rerun it. |
Collaborator
I try to take a look at it the next week. Checking it against the test suite as well |
Contributor
Author
|
@marci4 In the meantime, I have written a new RFC 7962 implementation from scratch, with tests validating all RFC 7962 examples, which I contributed to another project where I need it working: ChargeTimeEU/Java-OCA-OCPP#414 If interested, I could contribute the new implementation to this project as well. |
Collaborator
|
@robert-s-ubi feel free to contribute this as well |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Checklist: