Re: Memory Overright problem. (sorry if this is a repeat )

[Jan-Benedict Glaw <jbglaw@lug-owl.de>]

[-- Attachment #1: Type: text/plain, Size: 5593 bytes --]

On Wed, 2004-06-02 00:07:24 -0400, John T. Williams <jowillia@vt.edu>
wrote in message <1086149244.22758.17.camel@localhost>:
> #####################################################
> #include <string.h>
> 
> #ifndef NULL
> #define NULL (void*) 0
> #endif

There's a header file for NULL:)

> typedef struct _http {
>   char* query;	// query string
>   int  port;	// port number if other then 80
>   char* abspath;// absolute path of file on serve
>   char* host; 	// domain ex: www.something.com
> 
> } http_url;
> 
> int parsehttp( http_url*, char* );
> int initurl(http_url*);
> int cleanurl(http_url*);
> 
> int
> initurl( http_url* url)
> {
>   if( !url ) return -1;
>   url->host = NULL;
>   url->query = NULL;
>   url->abspath = NULL;
>   url->port = 80;
> }
> 
> int cleanurl(http_url* url) {
>   if( !url ) return -1;
>   if(url->host) {
>     free(url->host);
>     url->host = NULL;
>   }
>   if(url->query) {
>     free(url->query);
>     url->query = NULL;
>   }
>   if(url->abspath) {
>     free(url->abspath);
>     url->abspath = NULL;
>   }
>   url->port = 80;
> }
> 
> int
> parsehttp( http_url* http_addr, char* urlstr )
> {
>   char* host_ptr;
>   char* abspath_ptr;
>   char* port_ptr;
>   char* query_ptr;
> 
>   int host_len,
>     abspath_len,
>     port_len,
>     query_len,
>     total_len;
> 
> 
>   //check for null parameters:
>   if( !http_addr || !urlstr ) {
>     perror("error called parsehttp with NULL\n");

perror does only make sense if you call it after a system call, that
failed. In this case, I'd use fprintf (stderr, "message"); .

>     return -1;
>   }
> 
>   //check for http:// head
>   if( strncmp(urlstr, "http://", 7) ) {
>     perror("url not propperly formatted: %s\n", urlstr);
>     return -2;
>   }
> 
>   // find the starting point for each string
>   // if there indicating character is missing *ptr == NULL;
>   port_ptr = abspath_ptr = query_ptr = host_ptr = &urlstr[7];
>   while(port_ptr && *port_ptr != ':') port_ptr++; //search for ':'

So if there's no ':' (like in "http://somehost.com/index.html", you'll
see your first crash while overstepping the final '\0'. You probably
intended to do

	while (*port_ptr && *port_ptr != ':')
		port_ptr++;

This'll pay attention to the final zero.

> indicating
>   //					      		a port in this url
>   while(abspath_ptr && *abspath_ptr != '/') abspath_ptr++;
>   //					     	search for a '/' indicating
>   //						a absolute path listed
>   while(query_ptr && *query_ptr != '?') query_ptr++;
>   //						search for a '?' indicating
>   //						a query is listed

Same here.

>   // Parse out the query if any and record its length
>   if(*query_ptr) {
>     query_len = strlen(query_ptr);

It's still pointing to the leading '?' of the supplied arguments, right?

>     http_addr->query = (char*) malloc( sizeof( query_len + 128 ) );

Here's a servere one! It will probably allocate 4 bytes - query_len is
an integer! This should have been

	http_addr->query = (char *) malloc (query_len + 128);

>     strncpy(http_addr->query, &query_ptr[1], query_len - 1 );

...and a check for malloc's return value (if it failed, it might have
returned NULL) is missing, too. However, you're properly dealing with
the initial '?'.

>     http_addr->query[query_len - 1] = '\0';
>   } else query_len = 0;
>
>   // Parse out the abspath if any and record its length
>   if(*abspath_ptr) {
>     abspath_len = strlen(abspath_ptr) - query_len;
>     http_addr->abspath = (char*) malloc( sizeof( abspath_len + 128 ) );
>     strncpy(http_addr->abspath, abspath_ptr, abspath_len);

Same here - you allocated 4 bytes, forgot to check and write the full
path (which is probably longer than 3 bytes + '\0') to it.

>     http_addr->abspath[abspath_len] = '\0';
>   } else abspath_len = 0;
> 
>   // Parse out the port number if any
>   if(*port_ptr) {  // if a port was found
>     port_len = strlen( port_ptr) - abspath_len - query_len;
>     port_ptr++; //move past ':'
>     http_addr->port = atoi(port_ptr);
>   } else port_len = 0;
>   printf("port length: %i\n", port_len);
> 
>   // Parse out the host str if any
>   if(*host_ptr) {
>     host_len = strlen(host_ptr) - port_len - abspath_len - query_len;

This looks fragile. Don't ask me why, but I'd probably written that
using strcspn().

> /***************here********************/
>     http_addr->host = (char*) malloc( sizeof( host_len + 128 ) );
> /***************end here ***************/

You're only allocating 4 bytes and forget to check:)

>     strncpy(http_addr->host, host_ptr, host_len );
>     http_addr->host[host_len] = '\0';
>   }
> 
> 
>   return 0;
> }
> 
> 
> int
> main(int argC, char** argV, char** envp)
> {
>   http_url  url;
> 
>   initurl( &url );
>   parsehttp( &url,
> "http://www.vt.edu:23/users/jowillia/index.html?t=12");
> 
> 
>   printf("http://");
>   //  if(url.host) printf("%s", url.host);
>   if(url.port != 80) printf(":%i", url.port);
>   if(url.abspath) printf("%s", url.abspath);
>   if(url.query) printf("?%s", url.query);
>   printf("\n");
> 
>   cleanurl( &url );
> 
> 
>   return 0;
> }
> 
> 



-- 
   Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481
   "Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg
    fuer einen Freien Staat voll Freier Bürger" | im Internet! |   im Irak!
   ret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA));

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]