Skip to content

Fix issue #847#849

Merged
marci4 merged 3 commits into
TooTallNate:masterfrom
marci4:Issue847
Feb 5, 2019
Merged

Fix issue #847#849
marci4 merged 3 commits into
TooTallNate:masterfrom
marci4:Issue847

Conversation

@marci4
Copy link
Copy Markdown
Collaborator

@marci4 marci4 commented Jan 29, 2019

Description

This PR fixes the issue introduced with #805

Related Issue

Fixes #847

Motivation and Context

Bug fix

How Has This Been Tested?

Added a new test

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.

@marci4 marci4 added the Bug label Jan 29, 2019
@marci4 marci4 added this to the Release 1.4.0 milestone Jan 29, 2019
@PhilipRoman
Copy link
Copy Markdown
Collaborator

PhilipRoman commented Jan 30, 2019

Can you please explain what's happening in the Issue847Test? Why not simply send a large message and test it was received correctly?

@marci4
Copy link
Copy Markdown
Collaborator Author

marci4 commented Jan 31, 2019 •
edited
Loading

Can you please explain what's happening in the Issue847Test? Why not simply send a large message and test it was received correctly?

I was not able to reproduce with one single tcp packet.
Wireshark also showed that firefox is sending multiple tcp fragements and when recreating this behavior, the issue occured.

The relevant check to prevent this is translateSingleFrameCheckPacketSize, which ensures the buffer is big enough

PhilipRoman
PhilipRoman reviewed Feb 4, 2019
View reviewed changes
Comment thread src/main/java/org/java_websocket/drafts/Draft_6455.java
*/
private TranslatedPayloadMetaData translateSingleFramePayloadLength(ByteBuffer buffer, Opcode optcode, int oldPayloadlength, int maxpacketsize, int oldRealpacketsize) throws InvalidFrameException, IncompleteException, LimitExceededException {
int payloadlength = oldPayloadlength, realpacketsize = oldRealpacketsize;
int payloadlength = oldPayloadlength,
Copy link
Copy Markdown
Collaborator

@PhilipRoman PhilipRoman Feb 4, 2019

Choose a reason for hiding this comment

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

Are you sure that fixed the code style issue? 🤔 Probably meant to put them in separate declarations

Copy link
Copy Markdown
Collaborator Author

@marci4 marci4 Feb 5, 2019

Choose a reason for hiding this comment

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

Does the magic ;) (only when we are in a non strict mode according to the documentation)

@marci4 marci4 merged commit 564b8aa into TooTallNate:master Feb 5, 2019
@marci4 marci4 deleted the Issue847 branch February 5, 2019 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PhilipRoman PhilipRoman PhilipRoman left review comments

Assignees

No one assigned

Labels

Bug

Projects

None yet

Milestone

Release 1.4.0

Development

Successfully merging this pull request may close these issues.

java.nio.BufferUnderflowException

2 participants

@marci4 @PhilipRoman