CODEWITHSUNDEEP
c
vishalB
vishalB

Reputation: 13

How to concatenate two strings and save in third variable in C?

I have defined a char array with a predefined HTTP post string like this: 

char header[] = "POST /api/add HTTP/1.1\r\nHost: xxxxxxx:3000\r\nContent-Type: application/octet-stream; charset=utf-8\r\nContent-Length: 500\r\nName: ";

strcat(header, strDevicename); \\
strcat(header, "\r\n\r\n");

where the strDevicename is a char variable name changed every request. The problem is when I run it for the first time, it is working but after that getting overwritten Name with a96ed5ÿÿa96ed58e8355.

What is the best way to add two string with one real-time change variable using C languages in HTTP post header?

Upvotes: 1

Views: 1444

Answers (5)

0___________
0___________

Reputation: 67546

As you need to have enough space you can duplicate both in the new one:

char *mystrcat(const char *src1, const *char src2)
{
    char *dest
    size_t src1Len = strlen(src1);
    dest = malloc(src1Len + strlen(src2) + 1);
    if(dest)
    {
        memcpy(dest, src1, src1Len);
        strcpy(dest + src1Len, src2);
    }
    return dest;
}

Upvotes: 0

P.P
P.P

Reputation: 121397

The problem is that you are [str]concatenating to an array (header) which doesn't extra space (C arrays can't be changed in size).

snprintf could be a better fit here.

If you know the maximum possible length of strDevicename, you could use a fixed size buffer:

const char header[] = "POST /api/add HTTP/1.1\r\nHost: xxxxxxx:3000\r\nContent-Type: application/octet-stream; charset=utf-8\r\nContent-Length: 500\r\nName: ";
const char tail[] = "\r\n\r\n";
char buf[(sizeof header - 1) + MAX_DEV_LENGTH + (sizeof tail - 1) + 1];
/* sizeof would count the null byte in header & tail arrays. */
snprintf(buf, sizeof buf, "%s%s%s", header, strDevicename, tail);

If strDevicename is of unknown length, you could use asprintf(GNU function):

char *buf = NULL;
if (asprintf(&buf, "%s%s%s", header, strDevicename, tail) == -1) {
   /* handle memory allocation failure */
}

...

free(buf);

If you asprintf isn't available, then you could allocate sufficient memory (just as above) yourself using malloc and then use snprintf.

Upvotes: 0

Sourav Ghosh
Sourav Ghosh

Reputation: 134336

In your code, the size of the array header is decided by the size of the supplied initializer string, and it does not have any additional space to store (or append) any further characters.

Quoting C11, chapter §6.7.9

If an array of unknown size is initialized, its size is determined by the largest indexed element with an explicit initializer. The array type is completed at the end of its initializer list.

Next, for strcat(), from chapter §7.24.3.1

The strcat function appends a copy of the string pointed to by s2 (including the terminating null character) to the end of the string pointed to by s1. [...]

which indicates, the target s1 (here, header) should have sufficient storage to hold the concatenated string.

Thus, the attempt to strcat() with header as source, invokes undefined behaviour here, as you run past allocated memory.

You need to make header have enough space left after you fill it with the initializer string. Use a fixed size for the array, which has much excess after filling it with the initialize string, something like

#define STRSIZ 512

char header[STRSIZ] = "POST /api/add HTT.........

Upvotes: 4

Daniel Pryden
Daniel Pryden

Reputation: 60957

From man 3 strcat:

The strings may not overlap, and the dest string must have enough space for the result. If dest is not large enough, program behavior is unpredictable; buffer overruns are a favorite avenue for attacking secure programs.

You need to ensure your header array is allocated large enough to ensure that you can write strlen(strDeviceName) + 5 bytes into it in addition to the initial size; otherwise you have a (probably remotely exploitable) buffer overrun vulnerability.

Presumably header is allocated locally to the function? In that case you should consider using alloca or malloc to get a properly-sized block of memory rather than relying on a static size. You'll also need to handle errors from those functions.

Additionally, you should always prefer the safe alternative strncat over plain strcat, as strncat takes an additional argument for the number of bytes to append, and ensures that the buffer is always null-terminated even if an overflow would otherwise have happened.

Upvotes: 2

L. Scott Johnson
L. Scott Johnson

Reputation: 4382

You need to allocate sufficient space for the entire string. Man page:

char * strcat(char *restrict s1, const char *restrict s2);
The string s1 must have sufficient space to hold the result.

You could do it the "right" way and allocate exactly what you need:

char *buf;
char header[] = "POST /api/add HTTP/1.1\r\nHost: xxxxxxx:3000\r\nContent-Type: application/octet-stream; charset=utf-8\r\nContent-Length: 500\r\nName: "

buf = malloc(strlen(header)+strlen(strDeviceName)+strlen("\r\n\r\n")+1);
if(buf==NULL) abort();
strcpy(buf, header);
strcat(buf, strDevicename); \\
strcat(buf, "\r\n\r\n");

Or do it the lazy way and overallocate:

char header[1024] = "POST /api/add HTTP/1.1\r\nHost: xxxxxxx:3000\r\nContent-Type: application/octet-stream; charset=utf-8\r\nContent-Length: 500\r\nName: "

strcat(header, strDevicename); \\
strcat(header, "\r\n\r\n");

Upvotes: 0

Related Questions