fix:clientHbReq deserialize problem

[Pengrongkun]

Description

Issue(s)

• Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

• Is the user manual updated?
• Are the test cases passed and automated?
• Is there no significant decrease in test coverage?

[gemini-code-assist]

Summary of Changes

Hello @Pengrongkun, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a deserialization problem related to client heartbeat requests by enhancing the robustness and reliability of message processing. It introduces stricter validation for connection types, refines error handling to ensure proper resource release upon failure, and implements a length-prefixed serialization strategy for batch heartbeat requests. These changes collectively prevent malformed data from causing issues and ensure accurate parsing of batched requests, leading to a more stable system.

Highlights

• Improved Deserialization Robustness: Added validation for the connType field within SClientHbReq during deserialization, ensuring that connection types are within valid bounds to prevent processing of malformed messages.
• Enhanced Error Handling and Logging: Refactored error handling in tDeserializeSClientHbReq for memory allocation and array initialization failures, switching from direct returns to using TAOS_CHECK_GOTO to ensure proper resource cleanup. Additionally, a uError log message was added to the error handler for better debugging.
• Length-Prefixed Serialization for Batch Requests: Implemented a length-prefixed serialization and deserialization mechanism for SClientHbBatchReq. Each individual SClientHbReq within a batch is now preceded by its serialized length, which improves parsing reliability and prevents issues like over-reading during deserialization.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[Copilot]

Pull request overview

This PR fixes a deserialization problem in clientHbReq by adding validation for connection types, improving error handling consistency, and implementing length-prefixed serialization/deserialization to prevent corrupted data from affecting subsequent heartbeat requests in a batch.

Key Changes:

• Added validation to reject invalid connType values before processing
• Improved error handling by replacing direct returns with proper goto-based cleanup
• Implemented length-prefixed encoding/decoding for each heartbeat request within a batch to isolate parsing errors

---

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

[Copilot]

After deserializing each individual request with tDeserializeSClientHbReq, the memory allocated within that request needs to be freed if there's an error during the verification step (line 643-645). The function tDeserializeSClientHbReq allocates memory for pReq->query and pReq->info, but if reqDecoder.pos != reqLen check fails, these resources are not cleaned up before calling TAOS_CHECK_EXIT. This can cause a memory leak since tFreeClientHbReq is only called for items already added to the array.

Suggested change

	terrno = TSDB_CODE_INVALID_MSG;
	terrno = TSDB_CODE_INVALID_MSG;
	tFreeClientHbReq(&req);

[gemini-code-assist]

Code Review

This pull request addresses a deserialization issue with batched SClientHbReq messages. The fix involves prefixing each serialized request with its length, and then using a sub-decoder during deserialization to handle each request individually. This is a robust solution that prevents errors from cascading in a batch. The changes also include several other improvements, such as adding validation for connType, fixing potential memory leaks by consistently using goto for error handling, and adding more detailed error logging. Overall, these changes significantly improve the robustness and maintainability of the code. I have one minor suggestion to improve code clarity.

[gemini-code-assist]

The local variable code shadows the function-scoped variable of the same name. This can be confusing and lead to subtle bugs. It's better to use a different name for the local variable to improve clarity and avoid potential issues.

    int32_t ret = tSerializeSClientHbReq(&lenEncoder, pReq);
    int32_t reqLen = lenEncoder.pos;
    tEncoderClear(&lenEncoder);
    if (ret < 0) {
      TAOS_CHECK_EXIT(ret);
    }