riok / Kreya

Kreya is a GUI client for gRPC and REST APIs with innovative features for environments, authorizations and more.
https://kreya.app
284 stars 5 forks source link

Boolean fields with value `false` are ignored and not being added to payload when requesting. #88

Closed roku-on-it closed 2 years ago

roku-on-it commented 2 years ago

Describe the bug When false value is provided to a boolean field of an rpc method's input, Kreya is not adding that field to the request payload. Example 1: What I send from Kreya -> { "someBoolField": false, "hello": "world" } What I receive on the server -> { "hello": "world" }

Example 2: What I send from Kreya -> { "someBoolField": true, "hello": "world" } What I receive on the server -> { "someBoolField": true, "hello": "world" }

To Reproduce Steps to reproduce the behavior:

  1. Add proto file with an rpc method that has an input.
  2. Make sure input has a field type of bool
  3. Try to send false as that field's value
  4. Check your gRPC server and see the bool field is empty.

Expected behavior The field of type bool should be received as false but fields with value false are ignored and is not even sending to server.

Environment (if possible, copy the information from the error dialog or the About menu): { "kreyaVersion": "1.7.0", "platform": "Win32", "userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/102.0.5005.124 Safari/537.36 Edg/102.0.1245.44" }

Additional context I use NestJS microservices as my gRPC server.

I tested both with Postman and BloomRPC, when I try to send a bool field false or 0 I can see the value is coming to my server as false but when I send the same request with Kreya the input field is not added to payload.

CommonGuy commented 2 years ago

When using a scalar type such as bool, not serializing the default value is the expected behaviour (see the protobuf docs: https://developers.google.com/protocol-buffers/docs/proto3#default). When a gRPC server receives a scalar type without any value, it should set the field to the default value.

So this may be a bug in the NestJS gRPC server implementation. Could you provide a minimal .proto definition, so we can test this on our end?

roku-on-it commented 2 years ago

Sure, here is my protobuf definitions.

//user.proto

syntax = "proto3";

package user;

import "google/protobuf/empty.proto";
import "common.proto";

service UserService {
  rpc Register(CreateUser) returns (common.User);
  rpc Login(LoginInput) returns (LoginResponse);
}

message LoginResponse {
  string token = 1;
}

message CreateUser {
  string email = 1;
  string masterPassword = 2;
}

message LoginInput {
  string email = 1;
  string masterPassword = 2;
}
// settings.proto

syntax = "proto3";

package settings;

import "common.proto";

service SettingsService {
  rpc UpdateSettings (Update) returns (common.Settings);
}

message Update {
  bool theme = 1;
  repeated string ipWhitelist = 2;
}
// common.proto

syntax = "proto3";

package common;

message User {
  string id = 1;
  uint64 createdAt = 2;
  uint64 updatedAt = 3;
  optional uint64 deletedAt = 4;
  string email = 5;
}

message Settings {
  uint64 updatedAt = 1;
  bool theme = 2;
  repeated string ipWhitelist = 3;
}

The problem is with theme field on settings.proto. I came to a decision that Kreya was causing the issue because I tested in BloomRPC, Insomnia and Postman and all of three were working as expected except Kreya.

CommonGuy commented 2 years ago

The specification is pretty clear on how scalar types such as bool should be serialized:

Also note that if a scalar message field is set to its default, the value will not be serialized on the wire.

The NestJS gRPC server implementation violates the specification, as the spec states explicitely:

Note that for scalar message fields, once a message is parsed there's no way of telling whether a field was explicitly set to the default value (for example whether a boolean was set to false) or just not set at all

It shouldn't matter whether a boolean field is sent as false or not at all, the server should deserialize it as false.

Since Kreya behaves correctly according to the specification, we are not going to changes this.

Note: If you want to track whether a field has been set or not in proto3, you could use google.protobuf.BoolValue (a nullable wrapper around bool) or optional bool (supported as of v3.15 of protoc).

roku-on-it commented 2 years ago

Yeah, looks like I'll have to deal with NestJS. Thanks for your time.