grpc: add websocket support

[amitpanwar789]

Overview

Briefly describe the purpose, goals, and changes or improvements made in this pull request.

Related Issues

zaproxy/zaproxy#7695

Checklist

• Update help
• Update changelog
• Run ./gradlew spotlessApply for code formatting
• Write tests
• Check code coverage
• Sign-off commits
• Squash commits
• Use a descriptive title

For more details, please refer to the developer rules and guidelines.

[amitpanwar789]

@thc202 have a look here

[thc202]

The changelog should be updated and help as well.

[thc202]

How or with which application was this tested? Using streamlit I get e.g. Illegal base64 character a (though I might have wrong expectations of how that one works).

[amitpanwar789]

How or with which application was this tested? Using streamlit I get e.g. Illegal base64 character a (though I might have wrong expectations of how that one works).

@thc202
so i tried to test it with https://github.com/Cysharp/GrpcWebSocketBridge but get response as plain text. which app should i use to test it ? i tried a sample app of streamlit https://weaviate-magic-chat.streamlit.app/ . but the response i am getting in websocket is complex, i tried to decode with other protobuf decoder available online but nothing work.

[thc202]

I need to check suitable applications.

[amitpanwar789]

I need to check suitable applications.

@thc202
ok i get it why i am getting plan text as response, grpc don't work on websocket and browser does not support http2, for streaming data, client usually uses a websocket proxy to establish a connection between client and grpc server, proxy is sitting between client and server, and websocket connection only between client and proxy, Proxy: Translates WebSocket messages to gRPC calls and communicates with the gRPC server using HTTP/2. and zap is sitting between client and proxy and proxy is already decoding to message that's why zap get plain text as response . https://www.redhat.com/en/blog/grpc-anywhere

[thc202]

I don't think that all of them are using that workflow.

[thc202]

The streamlit is writing (similar for reading) the serialized proto buf messages directly to the WebSocket, based on:
https://github.com/streamlit/streamlit/blob/0dfe07d39e832b072771c52b0c11392dd2f8ca65/lib/streamlit/web/server/browser_websocket_handler.py#L63
https://github.com/streamlit/streamlit/blob/0dfe07d39e832b072771c52b0c11392dd2f8ca65/lib/streamlit/runtime/runtime_util.py#L78
https://googleapis.dev/python/protobuf/latest/google/protobuf/message.html#google.protobuf.message.Message.SerializeToString

[amitpanwar789]

The streamlit is writing (similar for reading) the serialized proto buf messages directly to the WebSocket, based on: https://github.com/streamlit/streamlit/blob/0dfe07d39e832b072771c52b0c11392dd2f8ca65/lib/streamlit/web/server/browser_websocket_handler.py#L63 https://github.com/streamlit/streamlit/blob/0dfe07d39e832b072771c52b0c11392dd2f8ca65/lib/streamlit/runtime/runtime_util.py#L78 https://googleapis.dev/python/protobuf/latest/google/protobuf/message.html#google.protobuf.message.Message.SerializeToString

yes, its not using base64 encoding that's why i am getting earlier error but still the response of streamit in websocket is not decoding correctly i tried to decode this in other online available protobuf decoder also, i guess i need to look more into streamit code.

[thc202]

We need to check how the serialization is being done in that case and update the code to handle that as well.

[amitpanwar789]

how can i find implementation of this function , i tried some ways but don't find it in streamlit and python-grpc repo

def SerializeToString(self, **kwargs):
    """Serializes the protocol message to a binary string.

    Keyword Args:
      deterministic (bool): If true, requests deterministic serialization
        of the protobuf, with predictable ordering of map keys.

    Returns:
      A binary string representation of the message if all of the required
      fields in the message are set (i.e. the message is initialized).

    Raises:
      EncodeError: if the message isn't initialized (see :func:`IsInitialized`).
    """
    raise NotImplementedError

[amitpanwar789]

i was confused about streamlit so i wrote a my custom app which uses websocket and protobuf serialization and what i found is we don't need to decode this message as base64 and this does not contain 5 byte header as with grpc-protocol buffer that's why we are getting error, my custom app is working great but still for streamlit message, some successfully decoded but some not,
need to remove this lines Base64.... and DecoderUtils.extractPayload(body), for websocket
but should i write custom view for websocket or we can find who is calling dataChanged so we can act accordingly.

 public void dataChanged(HttpPanelViewModelEvent e) {
        byte[] body = ((AbstractByteHttpPanelViewModel) e.getSource()).getData();
        httpPanelGrpcArea.setBorder(null);
        try {
            body = DecoderUtils.splitMessageBodyAndStatusCode(body);
            body = Base64.getDecoder().decode(body);
            byte[] payload = DecoderUtils.extractPayload(body);
            if (payload.length == 0) {
                httpPanelGrpcArea.setText("");
            } else {
                protoBufMessageDecoder.decode(payload);
                httpPanelGrpcArea.setText(protoBufMessageDecoder.getDecodedOutput());
            }
        } catch (Exception er) {
            httpPanelGrpcArea.setText(protoBufMessageDecoder.getDecodedOutput() + er.getMessage());
            httpPanelGrpcArea.setBorder(BorderFactory.createLineBorder(Color.RED));
        }
        if (!isEditable()) {
            httpPanelGrpcArea.discardAllEdits();
        }
    }

[thc202]

#5509 (comment)

That's depends on backend used https://github.com/protocolbuffers/protobuf/tree/main/python#implementation-backends
For upb I think it ends up calling https://github.com/protocolbuffers/protobuf/blob/a4f9ddd8fc2ff1afa6697059d494a9bf0c09c680/python/message.c#L1652 and https://github.com/protocolbuffers/protobuf/blob/a4f9ddd8fc2ff1afa6697059d494a9bf0c09c680/python/message.c#L1593

#5509 (comment)

We'll have to change HttpPanelGrpcView to allow to specify the decoding. Or fallback to direct decoding if not Base64.

[amitpanwar789]

#5509 (comment)

That's depends on backend used https://github.com/protocolbuffers/protobuf/tree/main/python#implementation-backends For upb I think it ends up calling https://github.com/protocolbuffers/protobuf/blob/a4f9ddd8fc2ff1afa6697059d494a9bf0c09c680/python/message.c#L1652 and https://github.com/protocolbuffers/protobuf/blob/a4f9ddd8fc2ff1afa6697059d494a9bf0c09c680/python/message.c#L1593

#5509 (comment)

We'll have to change HttpPanelGrpcView to allow to specify the decoding. Or fallback to direct decoding if not Base64.

ok are you suggesting to like pass a parameter like decodingMethod in constructor of HttpPanelGrpcView and then the dataChanged will look like this to handle base64 decoding but still need to figure out how to check if need to remove header or not DecoderUtils.extractPayload(body)

@Override
public void dataChanged(HttpPanelViewModelEvent e) {
    byte[] body = ((AbstractByteHttpPanelViewModel) e.getSource()).getData();
    httpPanelGrpcArea.setBorder(null);
    try {
        body = DecoderUtils.splitMessageBodyAndStatusCode(body);
        byte[] payload;
        if ("Base64".equalsIgnoreCase(decodingMethod)) {
            payload = Base64.getDecoder().decode(body);
        } else {
            payload = body;
        }
        if (payload.length == 0) {
            httpPanelGrpcArea.setText("");
        } else {
            protoBufMessageDecoder.decode(payload);
            httpPanelGrpcArea.setText(protoBufMessageDecoder.getDecodedOutput());
        }
    } catch (Exception er) {
        httpPanelGrpcArea.setText(protoBufMessageDecoder.getDecodedOutput() + er.getMessage());
        httpPanelGrpcArea.setBorder(BorderFactory.createLineBorder(Color.RED));
    }
    if (!isEditable()) {
        httpPanelGrpcArea.discardAllEdits();
    }
}

[thc202]

Yes, something like that but instead of a string use an enum (e.g. DecodingMethod with values BASE64_ENCODED, DIRECT). Doesn't that apply only if it was Base64 encoded?

At some point we might need to make that an option of the view to allow the user to change that.

[amitpanwar789]

Yes, something like that but instead of a string use an enum (e.g. DecodingMethod with values BASE64_ENCODED, DIRECT). Doesn't that apply only if it was Base64 encoded?

At some point we might need to make that an option of the view to allow the user to change that.

so after this check we are getting some response decoded properly and some not of streamlit Comment so , should i look more deeper for for streamlit now or is it good ?

[thc202]

It would be better to look at it (we should add an entry to the issue if not finished now), but we could merge and get feedback from users.

[thc202]

Note that comments from this #5509 (review) are still pending.

[thc202]

There's a conflict with the changelog.

[thc202]

Thank you!

[thc202]

@amitpanwar789 ok to release a new version or you want to include more changes?

[amitpanwar789]

@amitpanwar789 ok to release a new version or you want to include more changes?

Yes I guess we can as early feedback is really helpful