eBay / NuRaft

C++ implementation of Raft core logic as a replication library
Apache License 2.0
1.01k stars 240 forks source link

integer overflow ? #400

Open cbucher opened 1 year ago

cbucher commented 1 year ago
enum log_val_type {
    app_log         = 1,
    conf            = 2,
    cluster_server  = 3,
    log_pack        = 4,
    snp_sync_req    = 5,
    custom          = 999,
};

We can see that the "custom" entrie of the log_val_type enum is set to 999. However, when this value is serialized, it is cast as 8bit unsigned integer which maximum value is 255.

    ptr<buffer> serialize() {
        buff_->pos(0);
        ptr<buffer> buf = buffer::alloc( sizeof(ulong) +
                                         sizeof(char) +
                                         buff_->size() );
        buf->put(term_);
        buf->put( (static_cast<byte>(value_type_)) );
        buf->put(*buff_);
        buf->pos(0);
        return buf;
    }

Perhaps, the enum could be defined as :

enum log_val_type : byte {
    app_log         = 1,
    conf            = 2,
    cluster_server  = 3,
    log_pack        = 4,
    snp_sync_req    = 5,
    custom          = 999,
};
greensky00 commented 1 year ago

Hi @cbucher , thanks for bringing up this issue. Actually it was already reported before https://github.com/eBay/NuRaft/issues/349 and then forgotten.

I'd rather prefer changing it like this:

enum log_val_type : byte {
    app_log         = 1,
    conf            = 2,
    cluster_server  = 3,
    log_pack        = 4,
    snp_sync_req    = 5,
    custom          = 231,
};

given that 999 % 256 = 231. Would you like to submit a PR? Or let me know if you want me to do it.

cbucher commented 1 year ago

Hi,

Yes I will submit a PR.

cbucher commented 1 year ago

Hi @greensky00 , I have submitted a PR #404

Best regards