A message the parser cannot read costs that message and nothing after it - #109
Merged
Merged
Conversation
…tion When parseRelayMessage failed inside receive, the error returned before the reassembly buffer was cleared. The failed message stayed in it, every later frame was appended to it, and every message after it on that connection failed to parse too. The buffer is now cleared once a message is complete, whether it parses or not, and a message that cannot be read is skipped and counted. Relay.unreadable() reports how many. A deadline given to receiveTimeout still holds while messages are being skipped, since a relay streaming them never lets the socket run dry. Running out of memory inside the parser now comes back as OutOfMemory rather than InvalidMessage, so a skip never swallows a good message. Closes #108.
Carries #107 and the fix on this branch: an event that carries a key NIP-01 does not name parses, and a relay message the parser cannot read costs that message only.
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
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.
Closes #108.
When
parseRelayMessagefailed insidereceive, the error returned before the reassembly buffer was cleared. The failed message stayed inmsg, every later frame was appended to it, and every message after it on that connection failed to parse as well. Bytes that are not JSON, a message type the parser has no case for, or anEVENTwhose event is malformed were each enough.What changes
receivereads on.Relay.unreadable()reports the count, so a caller can say how many it dropped.receiveandreceiveTimeoutno longer returnerror.InvalidMessage; no caller I could find handled it by name.receiveTimeoutstill holds while messages are being skipped. The deadline is otherwise noticed only when the socket runs dry, and a relay streaming messages the parser cannot read never lets it run dry, so the skip checks it.OutOfMemoryinstead ofInvalidMessage, so a skip never swallows a good message.Tests
Each of these fails with its fix removed and passes with it:
a message the parser cannot read costs that message and nothing after itscripts not-JSON, an unknown type and a malformedEVENTahead of a NOTICE and an EOSE. It catches skipping without clearing, clearing without skipping, and the original clear-only-on-success.skipping unreadable messages still stops at the deadlinegives an already-passed deadline to a stream of unreadable messages and expectserror.Timeout, then the readable message on the next call.running out of memory is reported as that, not as a malformed messagefails every allocation of anEVENTparse in turn and requiresOutOfMemoryeach time, with nothing leaked. It uses a tag-heavy event, because a small one never reaches the backing allocator inside the event conversion and the inner mapping would go untested.parse EVENT refuses an event naming the same field twicepins the duplicate-key refusal on the relay path, which Parse an event that carries a key NIP-01 does not name #107 only pinned onfromJson.All 210 tests pass.
Release
The last commit bumps the version to 0.14.3 and moves both fixes, this one and #107, into the changelog.