Lipi C. H. Lee wrote: > implement simple 'wget' and port name can be specified in URL if default > port 80 is not used. > It may be added to toys/pending directory.
Thanks for your submission! Some comments below. In general, try simplifying the error messages: http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000850.html Also, generally functions are only added when they can be called at least twice. See http://lists.landley.net/pipermail/toybox-landley.net/2013-April/000919.html (URLs taken from http://landley.net/toybox/cleanup.html) > /* wget.c - Simple downloader to get the resource file in HTTP server > * > * Copyright 2016 Lipi C.H. Lee <[email protected]> > * > > USE_WGET(NEWTOY(wget, "f:", TOYFLAG_USR|TOYFLAG_BIN)) > > config WGET > bool "wget" > default n > help > usage: wget -f filename URL > -f filename: specify the filename to be saved > URL: HTTP uniform resource location and only HTTP, not HTTPS > > examples: > wget -f index.html http://www.example.com > wget -f sample.jpg http://www.example.com:8080/sample.jpg > */ > > #define FOR_wget > #include "toys.h" > > GLOBALS( > char *filename; > ) > > #define HN_LEN 128 // HOSTNAME MAX LENGTH > #define PATH_LEN 256 // PATH MAX LENGTH > > struct httpinfo { > char hostname[HN_LEN]; > char port[6]; // MAX port value: 65535 > char path[PATH_LEN]; > }; In the code base, global defines are usually avoided. If the functions were inlined, this struct would actually be unecessary and HN_LEN could be sizeof hostname. > > // extract hostname from url > static unsigned int get_hn(char *url, char *hn) { > unsigned int i; > > for (i = 0; url[i] != '\0' && url[i] != ':' && url[i] != '/'; i++) { > if(i >= HN_LEN) > error_exit("The hostname's length is lower than %d.", HN_LEN); You mean "larger" > hn[i] = url[i]; > } > hn[i] = '\0'; > > return i; > } > > // extract port number > static void get_port(char *url, char *port, unsigned int *url_i) { > unsigned int i; > > for (i = 0; url[i] != '\0' && url[i] != '/'; i++, (*url_i)++) { > if('0' <= url[i] && url[i] <= '9') port[i] = url[i]; > else error_exit("Port is invalid"); > } > if(i <= 6) port[i] = '\0'; > else error_exit("Port number is too long"); > } > > // get http infos in URL > static void get_info(struct httpinfo *hi, char *url) { > unsigned int i = 7, len; > > if (strncmp(url, "http://";, i)) error_exit("Only HTTP can be supported."); > len = get_hn(url+i, hi->hostname); > i += len; > > // get port if exists > if (url[i] == ':') { > i++; > get_port(url+i, hi->port, &i); > } else strcpy(hi->port, "80"); > > // get uri in URL > if (url[i] == '\0') strcpy(hi->path, "/"); > else if (url[i] == '/') { > if (strlen(url+i) < PATH_LEN) strcpy(hi->path, url+i); > else error_exit("The URL path's length is less than %d.", PATH_LEN); > } else error_exit("The URL is NOT valid."); > } > > // connect to any IPv4 or IPv6 server > static int conn_svr(const char *hostname, const char *port) { > struct addrinfo hints, *result, *rp; > int sock; > > memset(&hints, 0, sizeof(struct addrinfo)); > hints.ai_family = AF_UNSPEC; > hints.ai_socktype = SOCK_STREAM; > hints.ai_flags = 0; > hints.ai_protocol = 0; > > if ((errno = getaddrinfo(hostname, port, &hints, &result))) > error_exit("getaddrinfo: %s", gai_strerror(errno)); > > // try all address list(IPv4 or IPv6) until success > for (rp = result; rp; rp = rp->ai_next) { > if ((sock = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol)) > == -1) { > perror_msg("Socket Error"); > continue; > } > if (connect(sock, rp->ai_addr, rp->ai_addrlen) != -1) > break; // succeed in connecting to any server IP > else perror_msg("Connect Error"); > close(sock); > } > freeaddrinfo(result); > if(!rp) error_exit("Can not connect to HTTP server"); > > return sock; > } > > // make HTTP request header field > static void mk_fld(char *name, char *value) { > strcat(toybuf, name); > strcat(toybuf, ": "); > strcat(toybuf, value); > strcat(toybuf, "\r\n"); > } > > // make http request > static void mk_rq(char *path) { > strcpy(toybuf, "GET "); > strcat(toybuf, path); > strcat(toybuf, " HTTP/1.1\r\n"); > } Why not sprintf() for these? > > // get http response body starting address and its length > static char *get_body(size_t len, size_t *body_len) { > unsigned int i; > > for (i = 0; i < len-4; i++) > if (!strncmp(toybuf+i, "\r\n\r\n", 4)) break; > > *body_len = len-i-4; > return toybuf+i+4; > } Why not strstr()? > > void wget_main(void) > { > int sock; > struct httpinfo hi; > FILE *fp; > size_t len, body_len; > char *body, *result, *rc, *r_str, ua[18] = "toybox wget/", ver[6]; > > // TODO extract filename to be saved from URL > if (!(toys.optflags & FLAG_f)) > help_exit("The filename to be saved should be needed."); > if (fopen(TT.filename, "r")) > error_exit("The file(%s) you specified already exists.", TT.filename); fopen can fail for other reasons. Why not use perror_exit? > > if(!toys.optargs[0]) help_exit("The URL should be specified."); > get_info(&hi, toys.optargs[0]); > > sock = conn_svr(hi.hostname, hi.port); > > // compose HTTP request > mk_rq(hi.path); > mk_fld("Host", hi.hostname); > strncpy(ver, TOYBOX_VERSION, 5); > strcat(ua, ver); > mk_fld("User-Agent", ua); > mk_fld("Connection", "close"); > strcat(toybuf, "\r\n"); > > // send the HTTP request > len = strlen(toybuf); > if (write(sock, toybuf, len) != len) perror_exit("HTTP GET failed."); This gives something like: "HTTP GET failed.: Broken pipe" > > // read HTTP response > if ((len = read(sock, toybuf, 4096)) == -1) > perror_exit("HTTP response failed."); > if (!strstr(toybuf, "\r\n\r\n")) > error_exit("HTTP response header is too long."); > body = get_body(len, &body_len); > result = strtok(toybuf, "\r"); > strtok(result, " "); > rc = strtok(NULL, " "); > r_str = strtok(NULL, " "); > > // HTTP res code check > // TODO handle HTTP 302 Found(Redirection) > if (strcmp(rc, "200")) error_exit("HTTP response: %s(%s).", rc, r_str); > > if (!(fp = fopen(TT.filename, "w"))) perror_exit("File write error"); > if (fwrite(body, sizeof(char), body_len, fp) != body_len) sizeof(char)=1 > error_exit("File write is not successful."); > while ((len = read(sock, toybuf, 4096)) > 0) len is unsigned. So the case when read returns -1 is not correctly handled. > if (fwrite(toybuf, sizeof(char), len, fp) != len) > error_exit("File write is not successful."); > if (fclose(fp) == EOF) perror_exit("File Close Error"); > } --Felix _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
