Skip to content

Conversation

@Lorak-mmk
Copy link
Collaborator

If buffer is too short and we index past its end, code will panic. This is not desirable - we want to return errors on invalid protocol messages.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

@Lorak-mmk Lorak-mmk self-assigned this Oct 31, 2025
@Lorak-mmk Lorak-mmk added this to the 1.4.0 milestone Oct 31, 2025
@Lorak-mmk Lorak-mmk added bug Something isn't working area/deserialization labels Oct 31, 2025
@Lorak-mmk Lorak-mmk marked this pull request as ready for review October 31, 2025 15:28
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors deserialization code to use existing helper functions instead of manual buffer manipulation. The changes replace direct buffer slicing and advancement operations with calls to read_raw_bytes() and read_short_bytes(), improving code consistency and reducing duplication.

Key changes:

  • Replace manual buffer slicing with read_raw_bytes() helper in IP address deserialization
  • Replace manual length reading and slicing with read_short_bytes() in prepared statement ID parsing
  • Remove unused Buf trait imports from both modified files

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
scylla-cql/src/frame/types.rs Refactored read_inet() to use read_raw_bytes() helper; removed unused Buf import
scylla-cql/src/frame/response/result.rs Refactored deser_prepared() to use read_short_bytes() helper; removed unused Buf import
scylla-cql/src/frame/frame_errors.rs Added IdParseError variant to support the refactored error handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link

github-actions bot commented Oct 31, 2025

cargo semver-checks found no API-breaking changes in this PR.
Checked commit: 900a980

@Lorak-mmk Lorak-mmk force-pushed the scylla-cql-indexing branch 4 times, most recently from 55a793d to a84621d Compare October 31, 2025 15:49
@wprzytula wprzytula changed the title scylla-cql: Avoid index-acces to buffer scylla-cql: Avoid index-access to buffer Nov 1, 2025
If buffer is too short and we index past its end, code will panic. This
is not desirable - we want to return errors on invalid protocol
messages.
@Lorak-mmk Lorak-mmk force-pushed the scylla-cql-indexing branch from a84621d to 900a980 Compare November 3, 2025 09:00
@Lorak-mmk Lorak-mmk requested a review from wprzytula November 3, 2025 09:00
@Lorak-mmk Lorak-mmk merged commit 2dec09b into scylladb:main Nov 3, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/deserialization bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants