Skip to content

Conversation

@kozlovic
Copy link
Member

@kozlovic kozlovic commented Jan 7, 2026

It was reported that there was an issue in natsSock_ReadLine() which we use to read the initial INFO protocol. It is true that if reading the INFO would stop at the \r, just before the \n, then this function would not have detected the end of the protocl and would have waited for more data, which would have likely resulted in a timeout trying to connect.
So although possible, it is very unlikely that the read just ends one byte short, but regardless, this has now been fixed. The existing test has been made "real" with use of socket and not just read from prexisting buffer.

Resolves #958

Signed-off-by: Ivan Kozlovic [email protected]

It was reported that there was an issue in `natsSock_ReadLine()`
which we use to read the initial `INFO` protocol. It is true that
if reading the `INFO` would stop at the `\r`, just before the `\n`,
then this function would not have detected the end of the protocl
and would have waited for more data, which would have likely resulted
in a timeout trying to connect.
So although possible, it is very unlikely that the read just ends
one byte short, but regardless, this has now been fixed.
The existing test has been made "real" with use of socket and
not just read from prexisting buffer.

Resolves #958

Signed-off-by: Ivan Kozlovic <[email protected]>
@codecov
Copy link

codecov bot commented Jan 7, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.27%. Comparing base (6ba2a97) to head (fa8426f).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/comsock.c 50.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #959      +/-   ##
==========================================
+ Coverage   70.24%   70.27%   +0.02%     
==========================================
  Files          48       48              
  Lines       17357    17359       +2     
  Branches     3563     3563              
==========================================
+ Hits        12193    12199       +6     
+ Misses       1735     1731       -4     
  Partials     3429     3429              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kozlovic kozlovic requested a review from mtmk January 7, 2026 21:30
Copy link
Member

@mtmk mtmk left a comment

Choose a reason for hiding this comment

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

LGTM

@kozlovic kozlovic merged commit 86844ca into main Jan 8, 2026
45 of 46 checks passed
@kozlovic kozlovic deleted the fix_958 branch January 8, 2026 14:27
github-actions bot pushed a commit that referenced this pull request Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

strstr(p, "\r\n") in natsSock_ReadLine() searches only newly read bytes and can miss CRLF across read boundaries

3 participants