Commit 44128847 authored by Matt Caswell's avatar Matt Caswell
Browse files

Fix a bug in the new PACKET implementation 



Some of the PACKET functions were returning incorrect data. An unfortunate
choice of test data in the unit test was masking the failure.

Reviewed-by: default avatarTim Hudson <tjh@openssl.org>
parent 8d11b7c7

ssl/packet_locl.h

+8 −8
Original line number Diff line number Diff line
@@ -176,8 +176,8 @@ __owur static inline int PACKET_peek_net_3(PACKET *pkt, unsigned long *data) 
        return 0;



    *data  = ((unsigned long)(*pkt->curr)) << 16;

    *data |= ((unsigned long)(*pkt->curr + 1)) <<  8;

    *data |= *pkt->curr + 2;

    *data |= ((unsigned long)(*(pkt->curr + 1))) <<  8;

    *data |= *(pkt->curr + 2);



    return 1;

}
@@ -203,9 +203,9 @@ __owur static inline int PACKET_peek_net_4(PACKET *pkt, unsigned long *data) 
        return 0;



    *data  = ((unsigned long)(*pkt->curr)) << 24;

    *data |= ((unsigned long)(*pkt->curr + 1)) << 16;

    *data |= ((unsigned long)(*pkt->curr + 2)) <<  8;

    *data |= *pkt->curr+3;

    *data |= ((unsigned long)(*(pkt->curr + 1))) << 16;

    *data |= ((unsigned long)(*(pkt->curr + 2))) <<  8;

    *data |= *(pkt->curr+3);



    return 1;

}
@@ -254,9 +254,9 @@ __owur static inline int PACKET_peek_4(PACKET *pkt, unsigned long *data) 
        return 0;



    *data  = *pkt->curr;

    *data |= ((unsigned long)(*pkt->curr + 1)) <<  8;

    *data |= ((unsigned long)(*pkt->curr + 2)) << 16;

    *data |= ((unsigned long)(*pkt->curr + 3)) << 24;

    *data |= ((unsigned long)(*(pkt->curr + 1))) <<  8;

    *data |= ((unsigned long)(*(pkt->curr + 2))) << 16;

    *data |= ((unsigned long)(*(pkt->curr + 3))) << 24;



    return 1;

}

test/packettest.c

+25 −24
Original line number Diff line number Diff line
@@ -81,10 +81,10 @@ static int test_PACKET_get_1(PACKET *pkt, size_t start) 


    if (       !PACKET_goto_bookmark(pkt, start)

            || !PACKET_get_1(pkt, &i)

            ||  i != 0x01

            ||  i != 0x02

            || !PACKET_forward(pkt, BUF_LEN - 2)

            || !PACKET_get_1(pkt, &i)

            ||  i != 0xff

            ||  i != 0xfe

            ||  PACKET_get_1(pkt, &i)) {

        fprintf(stderr, "test_PACKET_get_1() failed\n");

        return 0;
@@ -99,10 +99,10 @@ static int test_PACKET_get_4(PACKET *pkt, size_t start) 


    if (       !PACKET_goto_bookmark(pkt, start)

            || !PACKET_get_4(pkt, &i)

            ||  i != 0x04030201UL

            ||  i != 0x08060402UL

            || !PACKET_forward(pkt, BUF_LEN - 8)

            || !PACKET_get_4(pkt, &i)

            ||  i != 0xfffefdfcUL

            ||  i != 0xfefcfaf8UL

            ||  PACKET_get_4(pkt, &i)) {

        fprintf(stderr, "test_PACKET_get_4() failed\n");

        return 0;
@@ -117,10 +117,10 @@ static int test_PACKET_get_net_2(PACKET *pkt, size_t start) 


    if (       !PACKET_goto_bookmark(pkt, start)

            || !PACKET_get_net_2(pkt, &i)

            ||  i != 0x0102

            ||  i != 0x0204

            || !PACKET_forward(pkt, BUF_LEN - 4)

            || !PACKET_get_net_2(pkt, &i)

            ||  i != 0xfeff

            ||  i != 0xfcfe

            ||  PACKET_get_net_2(pkt, &i)) {

        fprintf(stderr, "test_PACKET_get_net_2() failed\n");

        return 0;
@@ -135,11 +135,12 @@ static int test_PACKET_get_net_3(PACKET *pkt, size_t start) 


    if (       !PACKET_goto_bookmark(pkt, start)

            || !PACKET_get_net_3(pkt, &i)

            ||  i != 0x010203UL

            ||  i != 0x020406UL

            || !PACKET_forward(pkt, BUF_LEN - 6)

            || !PACKET_get_net_3(pkt, &i)

            ||  i != 0xfdfeffUL

            ||  i != 0xfafcfeUL

            ||  PACKET_get_net_3(pkt, &i)) {

        fprintf(stderr, "i is %ld\n", i);

        fprintf(stderr, "test_PACKET_get_net_3() failed\n");

        return 0;

    }
@@ -153,10 +154,10 @@ static int test_PACKET_get_net_4(PACKET *pkt, size_t start) 


    if (       !PACKET_goto_bookmark(pkt, start)

            || !PACKET_get_net_4(pkt, &i)

            ||  i != 0x01020304UL

            ||  i != 0x02040608UL

            || !PACKET_forward(pkt, BUF_LEN - 8)

            || !PACKET_get_net_4(pkt, &i)

            ||  i != 0xfcfdfeffUL

            ||  i != 0xf8fafcfeUL

            ||  PACKET_get_net_4(pkt, &i)) {

        fprintf(stderr, "test_PACKET_get_net_4() failed\n");

        return 0;
@@ -173,12 +174,12 @@ static int test_PACKET_get_sub_packet(PACKET *pkt, size_t start) 
    if (       !PACKET_goto_bookmark(pkt, start)

            || !PACKET_get_sub_packet(pkt, &subpkt, 4)

            || !PACKET_get_net_4(&subpkt, &i)

            ||  i != 0x01020304UL

            ||  i != 0x02040608UL

            ||  PACKET_remaining(&subpkt)

            || !PACKET_forward(pkt, BUF_LEN - 8)

            || !PACKET_get_sub_packet(pkt, &subpkt, 4)

            || !PACKET_get_net_4(&subpkt, &i)

            ||  i != 0xfcfdfeffUL

            ||  i != 0xf8fafcfeUL

            ||  PACKET_remaining(&subpkt)

            ||  PACKET_get_sub_packet(pkt, &subpkt, 4)) {

        fprintf(stderr, "test_PACKET_get_sub_packet() failed\n");
@@ -194,13 +195,13 @@ static int test_PACKET_get_bytes(PACKET *pkt, size_t start) 


    if (       !PACKET_goto_bookmark(pkt, start)

            || !PACKET_get_bytes(pkt, &bytes, 4)

            ||  bytes[0] != 1 || bytes[1] != 2

            ||  bytes[2] != 3 || bytes[3] != 4

            ||  bytes[0] != 2 || bytes[1] != 4

            ||  bytes[2] != 6 || bytes[3] != 8

            ||  PACKET_remaining(pkt) != BUF_LEN -4

            || !PACKET_forward(pkt, BUF_LEN - 8)

            || !PACKET_get_bytes(pkt, &bytes, 4)

            ||  bytes[0] != 0xfc || bytes[1] != 0xfd

            ||  bytes[2] != 0xfe || bytes[3] != 0xff

            ||  bytes[0] != 0xf8 || bytes[1] != 0xfa

            ||  bytes[2] != 0xfc || bytes[3] != 0xfe

            ||  PACKET_remaining(pkt)) {

        fprintf(stderr, "test_PACKET_get_bytes() failed\n");

        return 0;
@@ -215,13 +216,13 @@ static int test_PACKET_copy_bytes(PACKET *pkt, size_t start) 


    if (       !PACKET_goto_bookmark(pkt, start)

            || !PACKET_copy_bytes(pkt, bytes, 4)

            ||  bytes[0] != 1 || bytes[1] != 2

            ||  bytes[2] != 3 || bytes[3] != 4

            ||  bytes[0] != 2 || bytes[1] != 4

            ||  bytes[2] != 6 || bytes[3] != 8

            ||  PACKET_remaining(pkt) != BUF_LEN - 4

            || !PACKET_forward(pkt, BUF_LEN - 8)

            || !PACKET_copy_bytes(pkt, bytes, 4)

            ||  bytes[0] != 0xfc || bytes[1] != 0xfd

            ||  bytes[2] != 0xfe || bytes[3] != 0xff

            ||  bytes[0] != 0xf8 || bytes[1] != 0xfa

            ||  bytes[2] != 0xfc || bytes[3] != 0xfe

            ||  PACKET_remaining(pkt)) {

        fprintf(stderr, "test_PACKET_copy_bytes() failed\n");

        return 0;
@@ -239,16 +240,16 @@ static int test_PACKET_move_funcs(PACKET *pkt, size_t start) 
            ||  PACKET_back(pkt, 1)

            || !PACKET_forward(pkt, 1)

            || !PACKET_get_bytes(pkt, &byte, 1)

            ||  byte[0] != 2

            ||  byte[0] != 4

            || !PACKET_get_bookmark(pkt, &bm)

            || !PACKET_forward(pkt, BUF_LEN - 2)

            ||  PACKET_forward(pkt, 1)

            || !PACKET_back(pkt, 1)

            || !PACKET_get_bytes(pkt, &byte, 1)

            ||  byte[0] != 0xff

            ||  byte[0] != 0xfe

            || !PACKET_goto_bookmark(pkt, bm)

            || !PACKET_get_bytes(pkt, &byte, 1)

            ||  byte[0] != 3) {

            ||  byte[0] != 6) {

        fprintf(stderr, "test_PACKET_move_funcs() failed\n");

        return 0;

    }
@@ -289,7 +290,7 @@ int main(int argc, char **argv) 
    PACKET pkt;



    for (i=1; i<=BUF_LEN; i++) {

        buf[i-1] = i;

        buf[i-1] = (i * 2) & 0xff;

    }

    i = 0;