Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: still check request payload #229

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abel-von
Copy link
Contributor

the ttrpc golang do not send flagNodata in the request even it is an empty request.

fix #221

the ttrpc golang do not send flagNodata in the request even it is an
empty request.

Signed-off-by: Abel Feng <fshb1988@gmail.com>
@abel-von
Copy link
Contributor Author

/cc @wllenyj

@wllenyj
Copy link
Collaborator

wllenyj commented May 14, 2024

We don't have a test case to verify this ok yet. Previously I was doing compatibility test in my locally.
So some caution is needed.

Are there any compatibility issues between the current implementation and the Golang version?

@abel-von
Copy link
Contributor Author

The ttrpc-rust checks if there is no data in the request by FLAG_NO_DATA set. but golang version do not set this flag even it has no data

 let no_data = (req_msg.header.flags & FLAG_NO_DATA) == FLAG_NO_DATA;

so even it is an empty request with no data, because golang version do not set FLAG_NO_DATA, we will send an empty message to the stream handler, which is not expected.

        if !no_data {
            // Fake the first data message.
            let msg = GenMessage {
                header: MessageHeader::new_data(stream_id, req.payload.len() as u32),

@wllenyj
Copy link
Collaborator

wllenyj commented May 15, 2024

@Tim-Zhang Is there conflict this commit with the changes of #208 ?

@abel-von
Copy link
Contributor Author

It seems golang version of ttrpc fix the empty payload by checking if the client stream is set. Shall rust version follow that logic?

@abel-von
Copy link
Contributor Author

@abel-von
Copy link
Contributor Author

It seems we do not distinguish StreamingClient or StreamingServer in ttrpc-rust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ttrpc golang stream request with no FLAG_NO_DATA set
2 participants