Skip to content
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

PerMessageDeflateExtension#setDeflater()/#setInflater() is overwritten in case of no_context_takeover #1400

Open
robert-s-ubi opened this issue Feb 23, 2024 · 1 comment

Comments

@robert-s-ubi
Copy link

Describe the bug
In the PerMessageDeflateExtension class, the setDeflater() and setInflater() methods set the internal deflater and inflater objects. However, in case of client|serverNoClientTakeover being true (the latter is the default), the set objects will be overwritten internally by their hardcoded defaults. As this happens during data exchange, there is no obvious hook by which the application could repeat the setDeflater() and setInflater() calls.

Relevant excerpts from PerMessageDeflateExtension.java:

private boolean serverNoContextTakeover = true;
private boolean clientNoContextTakeover = false;

private Inflater inflater = new Inflater(true);
private Deflater deflater = new Deflater(Deflater.DEFAULT_COMPRESSION, true);

public void setInflater(Inflater inflater) {
this.inflater = inflater;
}

public void setDeflater(Deflater deflater) {
this.deflater = deflater;
}

@OverRide
public void decodeFrame(Framedata inputFrame) throws InvalidDataException {
...
// If context takeover is disabled, inflater can be reset.
if (clientNoContextTakeover) {
inflater = new Inflater(true);
}
...

@OverRide
public void encodeFrame(Framedata inputFrame) {
...
if (serverNoContextTakeover) {
deflater.end();
deflater = new Deflater(Deflater.DEFAULT_COMPRESSION, true);
}
...

Expected behavior
The Inflater and Deflater objects set once should be retained throughout the lifetime of a class instance. I'm not sure why the current implementation creates new objects instead of calling inflater.reset() and deflater.reset(), respectively. If the reset calls achieve the same result as recreating the objects, it is a simple two-line fix. If, however, new object instances are indeed required, the API would have to be changed to pass builder objects instead of object instances.

@robert-s-ubi
Copy link
Author

On further investigation, I found that actually none of the settings have any effect, because WebSocketImpl() calls copyInstance() on the PerMessageDeflateExtension instance, which creates a new object instance with the default settings.

Leading me to the conclusion that the second line in this excerpt of AutobahnServerTest also has no effect:

PerMessageDeflateExtension perMessageDeflateExtension = new PerMessageDeflateExtension();
**perMessageDeflateExtension.setThreshold(0);**
AutobahnServerTest test = new AutobahnServerTest(port, limit,
    new Draft_6455(perMessageDeflateExtension));

The threshold used will be 1024 despite this line. This is also what I observed when using the library in my code.

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

No branches or pull requests

1 participant