Re: ftp: drop needless strcspn
On Wed, Jun 28, 2023 at 07:01:34PM +0200, Omar Polo wrote: > On 2023/06/28 18:47:17 +0200, Theo Buehler wrote: > > On Wed, Jun 28, 2023 at 06:37:43PM +0200, Omar Polo wrote: > > > A similar thing could be applied to rpki-client' http.c as well, it > > > should work the same. > > > > Could you please send a diff? The files have diverged, but keeping the > > shared parts in sync is helpful. > > here it is, but i can't do more than compile-test it. > > The trailing spaces are trimmed in http_get_line(), so the strcspn is > just as redundant. Thanks. Works fine here and passes regress (well, except the one that's been annoying for way too long.
Re: ftp: drop needless strcspn
On Wed, 28 Jun 2023 18:37:43 +0200, Omar Polo wrote:
> since fetch.c revision 1.211 ("strip spaces at end of header lines and
> in chunked encoding headers") ftp removes trailing whitespaces early
> so we don't need to re-do that upon every header we parse.
>
> it can also be misleading since one can wonder why LAST_MODIFIED
> doesn't have " \t" but only "\t", or confront it with rpki-client'
> http.c and notice that there there is no strcspn() call in
> Last-Modified handling.
>
> I've tested it by modifying httpd(8) to append " \t " at the end of
> every header (legal per rfc 7230) and observing that ftp still works
> fine, including mtime handling.
OK millert@
- todd
Re: ftp: drop needless strcspn
On 2023/06/28 18:47:17 +0200, Theo Buehler wrote:
> On Wed, Jun 28, 2023 at 06:37:43PM +0200, Omar Polo wrote:
> > A similar thing could be applied to rpki-client' http.c as well, it
> > should work the same.
>
> Could you please send a diff? The files have diverged, but keeping the
> shared parts in sync is helpful.
here it is, but i can't do more than compile-test it.
The trailing spaces are trimmed in http_get_line(), so the strcspn is
just as redundant.
diff /usr/src
commit - 9832f5035d799ae12e9f848cff5650481b673260
path + /usr/src
blob - 4544eac237147fe77bcb9d0068438827ddd81424
file + usr.sbin/rpki-client/http.c
--- usr.sbin/rpki-client/http.c
+++ usr.sbin/rpki-client/http.c
@@ -1369,7 +1369,6 @@ http_parse_header(struct http_connection *conn, char *
else if (strncasecmp(cp, CONTENTLEN, sizeof(CONTENTLEN) - 1) == 0) {
cp += sizeof(CONTENTLEN) - 1;
cp += strspn(cp, " \t");
- cp[strcspn(cp, " \t")] = '\0';
conn->iosz = strtonum(cp, 0, MAX_CONTENTLEN, &errstr);
if (errstr != NULL) {
warnx("Content-Length of %s is %s",
@@ -1422,14 +1421,12 @@ http_parse_header(struct http_connection *conn, char *
sizeof(TRANSFER_ENCODING) - 1) == 0) {
cp += sizeof(TRANSFER_ENCODING) - 1;
cp += strspn(cp, " \t");
- cp[strcspn(cp, " \t")] = '\0';
if (strcasecmp(cp, "chunked") == 0)
conn->chunked = 1;
} else if (strncasecmp(cp, CONTENT_ENCODING,
sizeof(CONTENT_ENCODING) - 1) == 0) {
cp += sizeof(CONTENT_ENCODING) - 1;
cp += strspn(cp, " \t");
- cp[strcspn(cp, " \t")] = '\0';
if (strcasecmp(cp, "gzip") == 0 ||
strcasecmp(cp, "deflate") == 0) {
if (http_inflate_new(conn) == -1)
@@ -1439,7 +1436,6 @@ http_parse_header(struct http_connection *conn, char *
} else if (strncasecmp(cp, CONNECTION, sizeof(CONNECTION) - 1) == 0) {
cp += sizeof(CONNECTION) - 1;
cp += strspn(cp, " \t");
- cp[strcspn(cp, " \t")] = '\0';
if (strcasecmp(cp, "close") == 0)
conn->keep_alive = 0;
else if (strcasecmp(cp, "keep-alive") == 0)
Re: ftp: drop needless strcspn
On Wed, Jun 28, 2023 at 06:37:43PM +0200, Omar Polo wrote:
> since fetch.c revision 1.211 ("strip spaces at end of header lines and
> in chunked encoding headers") ftp removes trailing whitespaces early
> so we don't need to re-do that upon every header we parse.
>
> it can also be misleading since one can wonder why LAST_MODIFIED
> doesn't have " \t" but only "\t", or confront it with rpki-client'
> http.c and notice that there there is no strcspn() call in
> Last-Modified handling.
Who had this kind of confusion? :)
> I've tested it by modifying httpd(8) to append " \t " at the end of
> every header (legal per rfc 7230) and observing that ftp still works
> fine, including mtime handling.
>
> A similar thing could be applied to rpki-client' http.c as well, it
> should work the same.
Could you please send a diff? The files have diverged, but keeping the
shared parts in sync is helpful.
> As a bonus, drop the unused `s' variable in Retry-After handling.
That's also a leftover from r1.209 :)
ok tb
ftp: drop needless strcspn
since fetch.c revision 1.211 ("strip spaces at end of header lines and
in chunked encoding headers") ftp removes trailing whitespaces early
so we don't need to re-do that upon every header we parse.
it can also be misleading since one can wonder why LAST_MODIFIED
doesn't have " \t" but only "\t", or confront it with rpki-client'
http.c and notice that there there is no strcspn() call in
Last-Modified handling.
I've tested it by modifying httpd(8) to append " \t " at the end of
every header (legal per rfc 7230) and observing that ftp still works
fine, including mtime handling.
A similar thing could be applied to rpki-client' http.c as well, it
should work the same.
As a bonus, drop the unused `s' variable in Retry-After handling.
ok?
diff /usr/src
commit - 9832f5035d799ae12e9f848cff5650481b673260
path + /usr/src
blob - eb4c872b72b3db468f782a12756bb7f8fb64784d
file + usr.bin/ftp/fetch.c
--- usr.bin/ftp/fetch.c
+++ usr.bin/ftp/fetch.c
@@ -891,7 +891,6 @@ noslash:
if (strncasecmp(cp, CONTENTLEN, sizeof(CONTENTLEN) - 1) == 0) {
cp += sizeof(CONTENTLEN) - 1;
cp += strspn(cp, " \t");
- cp[strcspn(cp, " \t")] = '\0';
filesize = strtonum(cp, 0, LLONG_MAX, &errstr);
if (errstr != NULL)
goto improper;
@@ -964,10 +963,8 @@ noslash:
#define RETRYAFTER "Retry-After:"
} else if (isunavail &&
strncasecmp(cp, RETRYAFTER, sizeof(RETRYAFTER) - 1) == 0) {
- size_t s;
cp += sizeof(RETRYAFTER) - 1;
cp += strspn(cp, " \t");
- cp[strcspn(cp, " \t")] = '\0';
retryafter = strtonum(cp, 0, 0, &errstr);
if (errstr != NULL)
retryafter = -1;
@@ -976,7 +973,6 @@ noslash:
sizeof(TRANSFER_ENCODING) - 1) == 0) {
cp += sizeof(TRANSFER_ENCODING) - 1;
cp += strspn(cp, " \t");
- cp[strcspn(cp, " \t")] = '\0';
if (strcasecmp(cp, "chunked") == 0)
chunked = 1;
#ifndef SMALL
@@ -985,7 +981,6 @@ noslash:
sizeof(LAST_MODIFIED) - 1) == 0) {
cp += sizeof(LAST_MODIFIED) - 1;
cp += strspn(cp, " \t");
- cp[strcspn(cp, "\t")] = '\0';
if (strptime(cp, "%a, %d %h %Y %T %Z", &lmt) == NULL)
server_timestamps = 0;
#endif /* !SMALL */
